docs(witan): put the import-needs-omnigraph decision on record

[blarghmatey]

What are the relevant tickets?

tk-import-witan-server-needs-the-omnigraph-binary-s-e9e822 — filed as a question, not a bug report ("THIS MAY WELL BE INTENDED"). This answers it and takes the "IF IT IS NOT DONE" branch.

Description (What does it do?)

import witan.server raises RuntimeError: omnigraph binary not found on a machine that has installed witan-council but not run witan setup. bin/check_core_floor.py reported it on every run under UNRELATED, next to genuinely unexplained failures — so a reader had to re-derive whether it was a defect awaiting a fix, and the second or third time you do that you stop reading the section.

Which call actually raises. Not the OmnigraphClient constructor. Module-scope _ensure_graph(cfg.graph_uri) (server.py:155) takes its create path for a store that does not exist yet, and the _find_binary() there (server.py:140) has no degradation, unlike the re-apply path above it. Verified by importing in an env with PATH and HOME stripped; on the base commit the frames were server.py:131 → server.py:116 → omnigraph.py:2173 (the first two shift in this PR, since the docstring grew). The constructor two lines below _ensure_graph would raise for the same reason but never gets there — which matters, because it means a lazy client would not have fixed this.

Why it stays. Deferring the bootstrap to first use was weighed and declined. Importing this module IS a write and the CLI depends on it: witan.cli._common._srv diagnoses local-vs-deployed routing before importing, and hands local_dispatch.local_server the import as an unevaluated callable, so a refused write never reaches the store it is refusing to touch. That ordering is the #261 fix — a witan task close that printed Closed <slug>, exited 0, and wrote to a local store while the deployed graph still showed the task open nine days later.

The invariant is checkable in one line, "witan.server" not in sys.modules, and test_local_dispatch.py asserts exactly that — written after the weaker loader-injection test passed against a deliberately reintroduced regression. "The store is touched on first use" would not be checkable that way, because several legitimate paths reach the client without passing the guard (witan serve, the admin CLI commands, tests). A bright line around a failure mode that hid for nine days outweighs the import ergonomics it costs.

So:

• _ensure_graph's docstring carries the reasoning and names the guard it is load-bearing for.
• check_core_floor.py gets an EXPECTED bucket between FAIL and UNRELATED, printing the case with its reason. Exit status is unchanged — only floor still fails the check.
• Entries match on module and exception type and a message substring. Keyed on the module alone, a future unrelated RuntimeError from witan.server would inherit an explanation that does not apply to it, which is the failure mode an allowlist like this has.

No behaviour change.

How can this be tested?

just check-core-floor

Passes: witan-council … imported 49/49, witan-code … imported 30/30. Note this does not exercise the new branch on a developer machine — _find_binary falls back to ~/.local/bin/omnigraph when PATH misses, so the import succeeds and there is nothing to classify. The new branch only fires where the binary is absent, which is CI, and is what this PR's own core-floor job will show.

To exercise the classifier directly, the IMPORT_ALL snippet was extracted verbatim from the file and run against an env with PATH=/nonexistent and HOME pointed at a temp dir:

imported 48/49 modules
EXPECTED (by design; does not fail this check) witan.server: RuntimeError: omnigraph binary not found. Install via: witan setup
  > witan.server bootstraps the local store at module scope
    (`_ensure_graph`), which needs the omnigraph binary; `witan setup`
    installs it. Deliberate: the CLI's local-dispatch guard holds this
    import unevaluated precisely BECAUSE importing is what touches the
    store, and that ordering is the agent-kit#261 fix. See
    witan/server.py::_ensure_graph.
exit 0

just test-witan-council (984 passed), just check-versions, ./bin/gen_docs.py --check and prek all clean.

Additional Context

bin/ scripts have no test suite in this repo, so the classifier is verified by executing it rather than by a new test file.

A third option was considered and dropped: making OmnigraphClient resolve its binary on first use instead of in __init__. It is correct on its own merits — constructing a client is not using it — and would fix the store-exists-but-no-binary case. It does not touch the symptom here, since _ensure_graph raises first, so it would be churn bundled into a decision PR. Worth doing separately if anyone wants it.

[Copilot]

Pull request overview

Records the intentional import-time omnigraph dependency and clarifies it in core-floor diagnostics.

Changes:

• Documents why witan.server bootstraps storage during import.
• Classifies the known missing-binary failure as EXPECTED.
• Adds the decision to the witan changelog.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
mcp/servers/witan/witan/server.py	Documents import-time bootstrap rationale.
mcp/servers/witan/CHANGELOG.md	Records the design decision.
bin/check_core_floor.py	Reports the known failure separately without changing exit status.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.