fix(config): validate domain and state_dir - #195
Merged
Merged
Conversation
- domain accepts a bare host or a full URL, so the check is on what _derive_domain yields rather than on the form of the value - a hostless domain used to fail later in OswExpress.validate_domain, quoting a regex instead of naming OSW_DOMAIN - state_dir "~/osw" created a directory literally named "~", because Ledger builds its path with Path() and never expands it - a relative state_dir resolves against a working directory the MCP client chooses, so the ledger landed in an unpredictable place - last open item from #143
- reject "\osw-state" and "C:osw-state", non-absolute on both platforms - assert the expanduser RuntimeError becomes a named config error - state in the docs that a domain without a host is rejected
Contributor
Release previewNo version bump from the current commits (stays at Changelog preview (truncated)Preview via python-semantic-release and conventional commits. |
LukasGold
added a commit
that referenced
this pull request
Sep 21, 2026
* feat(mcp): add osw-mcp server exposing a live OSL instance Add an in-repo `osw[mcp]` extra and an `osw-mcp` stdio console script that wraps OswExpress and serves it over the Model Context Protocol for clients such as Claude Code. Tools: semantic/SPARQL/full-text search, category schema introspection, entity read + JSON-LD export, create/update/delete, full page-slot access, and file up/download. - Delete is provenance-guarded: a local JSON ledger records pages the server created/modified; deleting anything untracked requires confirm_external_delete=true. - Credentials resolve from env/.env and are validated up front (fail fast, never prompts, so stdio is never corrupted by an input() call). - osw stdout is redirected to stderr so it never leaks onto the JSON-RPC channel. - OSW_MCP_READ_ONLY hides all mutating tools. * feat(mcp): port server to mcp 2.x and isolate the extra - FastMCP replaced by MCPServer, extra now requires mcp>=2 - mcp dropped from the all extra and the dev group: it needs anyio>=4.9, workflow pins anyio<4.7 (#139) - uv conflicts declare mcp exclusive with workflow and with dev - pytest stack moved to its own test group, so an environment with both pytest and mcp exists - src/osw/mcp excluded from ty, mcp tests guarded by importorskip * feat(mcp): authenticate from an osw credential file - OSW_MCP_CRED_FILEPATH configures it, OSL_CRED_FILEPATH is a fallback - an alternative to OSW_USERNAME/OSW_PASSWORD, so the password is not duplicated into a second plaintext file - existence and a matching domain entry are validated at startup - lookups use CredentialFallback.none, so osw never prompts and never blocks the stdio transport * feat(mcp): select between multiple OSL instances at runtime - list_instances and select_instance tools, returning iris only and never any credential value - OSW_DOMAIN becomes optional when a credential file supplies the iris - auto-selects when OSW_DOMAIN is set or the file holds exactly one iri - switching rebuilds the connection and the per-domain provenance ledger - tools resolve the active domain and credentials at call time * refactor: extract SDK-free osw.service core from osw.mcp - move config/ledger/serialization from osw.mcp to osw.service - add errors.py (OpError), context.py (Context+Policy), registry.py - Operation validator rejects path-like params on the mcp surface - canonical OSW_* env names, OSW_MCP_*/OSL_* kept as aliases - config/ledger/serialization tests now run without the mcp extra * refactor(service): lift search tools into osw.service.ops - add osw/service/ops/ with the search group as @operation functions - mcp/tools/search.py becomes a registry loop over bind() - add transitional legacy_context() so existing tests keep passing - bind() resolves annotations against the op module, not registry.py * refactor(service): lift remaining tool bodies into osw.service.ops - schema, status, entities and slots lifted as @operation functions - mcp/tools/*.py collapse to registry loops over bind() - error dicts become raised errors.*; ledger calls become records= hooks - normalize cli_name so every group reads as `osw <group> <verb>` - 172 passed in the dev env, 29 in the mcp extra env * feat(cli): add typer CLI assembled from the operation registry - typer as a base dependency; osw = "osw.cli.main:app" console script - commands built from iter_operations(surface="cli"); Context built lazily - OpError.exit_code becomes the process exit status, no traceback - json_value parser lives in osw.service.params, so core never imports cli - set_slot coerces content per the sibling slot's content model * feat: remove filesystem paths from the MCP surface - add path-free get_file_info/read_file_text/write_file_text built on WikiFileController.get()/.put(), never touching the local filesystem - move file download/upload and ledger path to osw.cli.ops, the only module allowed to name a path (surfaces={"cli"}); drop status's ledger_path - guard tests: no MCP-surfaced op names a path, and osw.mcp.server never imports osw.cli - read_file_text decodes incrementally so a byte cap splitting a multi-byte character is not misreported as binary content * feat(mcp): pin one instance per server, wire annotations and meta - delete list_instances/select_instance; each server process is pinned to one OSL instance and refuses to start if none resolves - map Operation's hints onto ToolAnnotations and _meta by explicit keyword, since the SDK silently absorbs a misspelled field name - set server instructions and version; make mcp.run(transport="stdio") explicit - add CLI --instance and osw instance list, returning iris only - retarget the "no instance selected" message at OSW_DOMAIN and --instance instead of the removed tool * refactor(mcp): drop tool closures, fold registration into server - delete osw/mcp/tools/ and connection.py; bodies live in osw.service.ops - fold registration.py into server.py, its sole consumer - replace test_mcp_tools.py with test_service_ops.py + test_mcp_server.py - rebuild the integration fixture on bind()/iter_operations - test_mcp_instances.py -> test_service_instances.py, no longer SDK-gated * build: type-check src/osw/mcp instead of excluding it - drop src/osw/mcp from [tool.ty.src] exclude - silence only the two unresolvable SDK imports inline (issue #139) * docs: document the osw CLI and unify the config reference - add a Command line section: command tree, global options, exit behaviour - move credentials into a shared Configuration section with an alias table - state that no MCP tool takes a path, and where the path-based commands live - add stdio type, multi-instance and per-instance permission examples - give each CLI command group a one-line help string * fix(config): find .env from the CWD, report config sources - CLI now searches upward from the working directory, not from the installed package's directory (dotenv's default walks the call stack) - MCP server searches nowhere: its CWD is chosen by the client - both print the resolved .env and credential file to stderr at startup - a missing credential file whose path holds a control character now explains .env double-quote escape decoding * docs: move CLI and MCP sections out of the README - new docs page "CLI and MCP tools", added to the zensical nav - README keeps a short pointer section, otherwise back to its old shape - get-started extras table gains the osw[mcp] row * docs: default MCP examples to a credential file in env - server entries now set OSW_CRED_FILEPATH plus OSW_DOMAIN, no .env needed - multi-instance example shares one credential file, one domain per server - register via claude mcp add-json, which takes the entry verbatim * feat(mcp): require an explicitly configured OSW_DOMAIN - server no longer auto-selects a single-iri credential file; the CLI still does - docs present the env block and the .env file as two supported styles - multi-instance example shows one server of each style - drop the "OSW_DOMAIN may be omitted" note * docs: restructure the CLI and MCP guide - add a Setup section with the uv/pip installs up front - move Configuration below the MCP section, both adapters share it - state where the .env is looked for first, drop the always-loaded claim - note that --instance is optional and when it is required - collect the rationale in a Design notes section at the end * docs: simplify the CLI and MCP setup section - lead with uv tool install, the mcp extra includes the base package - fold the pip, uv add and uvx variants into a details element - move the editable-install caveat to a Notes for developers section - document running the server from a local checkout via uvx --from * added local folders to .gitignore * docs: correct why the CLI may infer an instance - --instance is optional, so it is not what makes inference acceptable - the CLI resolves per invocation and reports the instance it resolved - state the --instance condition as OSW_DOMAIN unset, not .env-specific * refactor: de-isolate the mcp extra from the dev environment - drop the ty: ignore on the mcp SDK imports in server.py - run the MCP tests unconditionally instead of importorskip-ing them - update the config.py env-file hint (no more `test` group) - docs: mcp is part of osw[all]; replace the anyio design note * docs: drop remaining references to the separate MCP environment - test docstrings no longer contrast against a plain dev env - deptry comment no longer claims extras are absent from dev - README lists osw[mcp] among the extras * refactor(service): validate Settings with pydantic - convert Settings from a frozen dataclass to a frozen pydantic model - add validators for domain, sparql_endpoint, state_dir, cred_filepath - constrain max_results/max_chars to positive integers via Field(gt=0) - drop _int_env in favour of one ValidationError -> RuntimeError site that still names the exact alias that was set Closes #143 * test: stop test_init_from_env_vars leaking OSW_CRED_FILEPATH - use monkeypatch.setenv so OSW_CRED_FILEPATH and OSW_DOMAIN are restored - the test unlinks its credential file, so the leaked path pointed every later test at a missing file - surfaced by the mcp de-isolation: tests/integration/test_mcp_server.py no longer skips for a missing SDK, so it hit the polluted environment * test: do not assume the first ask-query hit carries jsondata - SMW ask results have no defined order and Category:Item can hold pages without a jsondata slot - scan all returned titles, require at least one with the slot * fix(service): validate read_only via pydantic instead of truthy set - route OSW_READ_ONLY through Settings so an unparseable value raises - a typo like "ture" previously yielded False, silently enabling writes - "y"/"t" now parse as true; all documented spellings keep working - blank/whitespace-only still falls back to the default, as for the ints - drop the now-unused _TRUTHY set Follow-up to #143. * fix(deps): make python-dotenv a base dependency - the osw CLI is a base console script, but .env loading required osw[mcp] - without dotenv the implicit .env search returned silently, so the CLI reported missing credentials instead of the missing package - drop the now redundant entry from the mcp extra - update the docstrings and error message that named osw[mcp] * feat(cli): find accounts.pwd.yaml in the working directory - look for accounts.pwd.yaml in the CWD when no credential file is set - skip the lookup when OSW_USERNAME or OSW_PASSWORD is configured - ignore a discovered file with no entry for the configured domain - report the credential file and where its path came from - no parent-directory walk, and the MCP server never searches * feat(cli): print one configuration line by default - reduce the CLI banner to the credential source unless --verbose - print the env-file line too when a command fails - omit the MCP stdio hint from CLI error messages - flush the banner so it survives a buffered stderr * docs(mcp): add a quick install section for claude mcp add - one claude mcp add command with the env block inline - note on quoting osw[mcp] and on Windows paths - scope flags, and how to list, inspect and remove the entry * docs(cli): make the search help explain OSW-ID titles - rename full_text_search to search_titles: it searches titles only - drop "full-text" from the group help, module docstring and docs - state that OSW pages are titled by OSW-ID, so names are not in titles - add a wildcard label example to `search ask`, with the caveat that the property name depends on the instance schema (not run against a wiki) - document the --limit default and the {titles, count, truncated} shape * feat(search): add a page-content search - wiki_tools.content_search wraps the MediaWiki list=search API - WtSite.content_search delegates to it, like prefix_search - new operation search_content, CLI `osw search content`, MCP tool - returns page titles in the same shape as the sibling searches * docs(cli): rename `search text` to `search titles` - `text` next to the new `content` implied it searched page text - ground the name-property advice in Entity.json's @context mapping - name the jsondata slot in the content-search caveat - list `titles` and `content` in the command table * docs(mcp): describe the credential lookup without the rationale - replace the env-file prose with a CLI/server table - state the credential-file order as steps, drop the reasons - merge the duplicated credential setup into one section - fix quick install: osw-mcp is the console script, not the server name - correct the default `claude mcp add` scope to local * feat(mcp): report config sources only when OSW_VERBOSE is set - add Settings.verbose, read from OSW_VERBOSE or OSW_MCP_VERBOSE - MCP server buffers the credential and env-file lines instead of writing them to stderr at every start - a failed start still prints both lines, verbose or not - CLI output is unchanged * test(mcp): cover main() on a successful start, quiet and verbose * fix(service): flag truncation at the limit that reached the wiki - cap_list only reports a cut it made itself, but all four search operations pass the same limit to the wiki, which never returns more, so `truncated` was always False - flag when the result set is as large as the effective limit - search_entities: a `limit=` inside the ask query wins over the limit argument, so compare against that one - sparql_query unchanged; its limit applies to the response, not the query * docs(service): say what search_entities actually counts * docs: reorganize CLI and MCP pages into a Tools section - replace the "CLI and MCP tools" tab with a "Tools" tab - split the single page into Overview, CLI, MCP and Configuration - leave room for a User Sync page from PR #174 - repoint the links in README.md and get-started.md * feat(cli): add instances status, fix the username status reports - status reports the username from a credential file, not only from OSW_USERNAME, and uses the precedence the login path uses - rename the "instance" group to "instances", unconfusable with --instance - add "osw instances status" to check every configured instance at once - name the correct form when a root option is typed after the command - extract config.get_credentials_for and Context.osw_for * fix(service): print the running adapter's name in shared messages - shared code hardcoded [osw] or [osw-mcp], so each adapter printed the other one's name about half the time - config.set_log_prefix/log_prefix; CLI sets osw, MCP server sets osw-mcp - covers the default delete and set_slot wiki edit comments, which are written permanently into page histories - add flush=True to the status connection-failure print, which CliRunner otherwise never captures because it flushes only stdout * refactor(cli): rename osw search instances to osw search entities - "instances" read as OSL instances, which is what osw instances lists - MCP tool name is the function name, so list_instances_of_category becomes search_entities and the ask query becomes search_ask - fix the help text, which documented [[HasType::Category:<category>]] while the real query for Category:Item is [[HasType::Category:Item]] - set the MCP prefix in the integration fixture that stands in for the server, so a future write test cannot tag a live wiki edit as [osw] * test(logging): reset the osw logger in its fixture The tests assumed osw was imported after something had configured logging, which decides whether osw attaches its own handler on import. tests/conftest.py now imports osw, so that assumption no longer held and the hand-over test read the leftover import-time handler. * refactor(service): log shared diagnostics instead of printing them - six stderr prints in the shared code move to the osw logger - removes messages lost to an unflushed stderr write - the configuration source report stays a print, gated by --verbose - the MCP startup failure stays a print, so OSW_LOG_LEVEL=OFF cannot hide it * build: declare click as a direct dependency osw.cli.main subclasses click's Command and Group to report a misplaced root option, so click is imported directly. deptry flagged it as DEP003 and failed the quality job. * docs(service): correct the reason the config report flushes sys.stderr is line-buffered on every supported Python (>=3.10). The real reason is the caller-chosen stream: the MCP server passes an io.StringIO and click's CliRunner never flushes its stderr replacement. * test(cli): normalize typer's rich output before asserting - typer forces colour on when GITHUB_ACTIONS is set - rich styles an option's leading dash separately, so "--jsondata" is no longer a plain substring - rich wraps at the real terminal width, 80 in CI, 79 on Windows - strip the escape sequences and panel borders, then join the lines * fix(cli): write stdout and stderr as UTF-8, not the locale encoding - a redirected stream used cp1252 on Windows, so a German label reached the consumer as bytes no JSON parser could read, with exit code 0 - a character cp1252 cannot represent raised UnicodeEncodeError instead - the app callback reconfigures both streams before any output - errors= is passed too, or reconfigure would reset stderr to strict - --help and an unknown command name still bypass it; neither carries wiki content Closes #187 * feat(cli): suppress osw's import notice on the console scripts - osw and osw-mcp now start through the new src/osw_entry.py shim - the shim sets OSW_LOG_LEVEL before osw is imported, which is the only place that can, since importing any submodule imports the package first - setdefault leaves a value the caller already chose alone - a plain "import osw" still writes the notice, library behaviour is deliberately unchanged - the wheel ships osw_entry.py via only-include plus sources * test(cli): cover the osw-mcp script and the shim's level choice - the osw-mcp console script had no test of its own, only osw did - it now runs as a subprocess and must leave stdout empty, which is the assertion that matters for a JSON-RPC transport - the level the shim sets moved into _DEFAULT_LEVEL, and a test ties it to osw.DEFAULT_LOG_LEVEL so the two cannot drift apart * fix(config): validate domain and state_dir (#195) * fix(config): reject a hostless domain and a relative state_dir - domain accepts a bare host or a full URL, so the check is on what _derive_domain yields rather than on the form of the value - a hostless domain used to fail later in OswExpress.validate_domain, quoting a regex instead of naming OSW_DOMAIN - state_dir "~/osw" created a directory literally named "~", because Ledger builds its path with Path() and never expands it - a relative state_dir resolves against a working directory the MCP client chooses, so the ledger landed in an unpredictable place - last open item from #143 * test(config): cover drive-relative state_dir and an unknown home - reject "\osw-state" and "C:osw-state", non-absolute on both platforms - assert the expanduser RuntimeError becomes a named config error - state in the docs that a domain without a host is rejected * test: decode subprocess output as UTF-8, not the locale codepage - text=True alone decodes with locale.getencoding(), which ignores Python's UTF-8 mode, so a child writing UTF-8 broke the reader thread with UnicodeDecodeError and returned the stream as None - reproduced on Windows: byte 0x90 at position 1028 of osw --help - the osw_entry tests now share a _DECODE mapping - test_no_paths_on_mcp_surface gets the same kwargs as protection
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
1 participant
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.
Last open item from #143.
Settingsbecame a pydantic model earlier on this branch,_int_envis gone, andsrc/osw/mcpis type-checked again. What remained is the issue's fourth point:domain,sparql_endpointandstate_dirwere unvalidated.sparql_endpointalready has a scheme and netloc check, so this covers the other two.domain
Validates the outcome, not the form. The validator runs the value through
_derive_domainand rejects it when no host comes out.wiki.example.org,https://wiki.example.org/w/https://,//,https:///w/,/w/index.phpBoth forms stay legal deliberately.
_derive_domain(src/osw/service/config.py:196) exists to reduce either one to a bare host, andtest_domain_as_full_url_acceptedholds that contract. Every call site that needs a bare host goes throughget_active_domain()first, never throughsettings.domain.OswExpress.validate_domain(src/osw/express.py:68) rejects a hostless value too, but only on the first connection, and its message quotes a regex.config.load()now reports it at startup instead:Not tightened to the
OswExpressregex, which requires a dot and a TLD. That would rejectlocalhostand any host carrying a port.state_dir
Two changes:
~is expanded.Ledger.__init__builds its path asPath(state_dir) / ...(src/osw/service/ledger.py:70), and nothing expanded a user-supplied value, sostate_dir="~/osw"created a directory named literally~.expanduser()was applied only when computing the default location.osw-mcpthat directory is chosen by the MCP client, so the ledger landed somewhere the user did not intend.This is the first validator on
Settingsthat rewrites its value; the other four only accept or reject. The alternative, rejecting~and requiring the full path, is worse for the operator writing a config file.Not added: an existence or writability check.
Ledger._save()creates the directory lazily on first write, so requiring it at config load would break that.Tests
Twelve cases in
tests/test_service_config.py:test_domain_without_a_host_rejected, four parameterstest_state_dir_expands_a_leading_tildetest_state_dir_relative_rejected, five parameterstest_state_dir_absolute_is_left_alone, the regression guard for the unchanged pathtest_state_dir_reports_an_undeterminable_homeThe first eight were written first and watched fail. Two of the five relative parameters and the undeterminable-home case were added afterwards, so each was checked by removing the production code it guards:
RuntimeErrorcatch,test_state_dir_reports_an_undeterminable_homefails with a bareRuntimeErrorinstead of aValidationErrorvalue.startswith(("/", "\\")) or ":" in valuecheck in place ofPath.is_absolute(), the\osw-stateandC:osw-stateparameters pass through and their test fails, while the other three still pass\osw-state(rooted, no drive) andC:osw-state(drive, no root) both resolve against process state, so both are rejected.is_absolute()returnsFalsefor them on Windows and on POSIX, so the parameters need no platform marker.All 591 unit tests pass.
tests/integration/test_mcp_server.py:28setsOSW_MCP_STATE_DIRto an absolute path, so the integration suite is unaffected.Docs
The
OSW_DOMAINandOSW_STATE_DIRrows of the environment variable table indocs/tools/configuration.mdnow state what each accepts, and that a domain with no host is rejected at startup.Follow-up this creates
OSW_STATE_DIRexpands~after this change andOSW_CRED_FILEPATHdoes not, so the two path settings now disagree. A tilde credential path fails with "does not exist" although the file exists. Filed as #194 rather than fixed here, because issue #143 does not name that setting. The failure is loud, so no credential is read from the wrong place.