From 5269f160de4e85b5e03492e809ec2b2e239ab774 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Wed, 19 Aug 2026 10:59:43 -0400 Subject: [PATCH 1/2] docs: give witan-core its own page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit witan-core was the one third of the documented trio with no page. It appeared only as a row in an architecture table and a line in the package list, so a reader wanting to know what it provides had to go read its README in the repo — which is the gap the site exists to close. Written as explanation rather than generated API reference, deliberately. witan-core describes itself as "shared core for the witan MCP servers": it is an internal dependency, not a library third parties build against, and a symbol-by-symbol reference would add a large low-traffic section while implying a stability promise the package does not make. It would also mean adopting mkdocstrings, giving up the site's current no-plugin property — the thing keeping a fall back to mkdocs-material cheap while Zensical is pre-1.0. What the page covers is what a contributor actually needs: - **Why it exists.** Not "shared code" in the abstract, but the specific class of code where two copies drifting is a *correctness* bug: the repo-key canonicaliser, whose output is the join key between the memory and code graphs, and the pinned omnigraph binary version, where a mismatch is a strict-format error rather than an inconsistency. - **The leaf invariant.** witan_core imports neither server, which is what keeps the optional `witan` → `witan-code` mount a DAG instead of a cycle. - **Why the base package is stdlib-only**, and why `observability` and `sentry` are separate extras rather than one. - **The version-floor trap**, which is the thing that will actually bite a contributor. The uv workspace resolves witan-core by path, so adding a witan_core symbol and using it in a server passes locally and in CI, then fails at `pip install` time for anyone resolving from PyPI. Nothing automated catches it — the path resolution is precisely what hides it — and the pin comments record it having happened five times in witan-council and three in witan-code. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017oVnpzjRJQVWq9hJGE2Rs7 --- docs/explanation/index.md | 5 ++ docs/explanation/witan-core.md | 136 +++++++++++++++++++++++++++++++++ docs/index.md | 2 +- zensical.toml | 1 + 4 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 docs/explanation/witan-core.md diff --git a/docs/explanation/index.md b/docs/explanation/index.md index 6ef83d06..b120fd14 100644 --- a/docs/explanation/index.md +++ b/docs/explanation/index.md @@ -25,6 +25,11 @@ to work against the grain of one. Symbol identity, edge precision tiers, and how the cross-repo bridge is stitched together. +- **[witan-core](witan-core.md)** + + The shared floor under both servers: what belongs there, why it is + stdlib-only, and the version-floor trap the uv workspace hides. + ## Decisions diff --git a/docs/explanation/witan-core.md b/docs/explanation/witan-core.md new file mode 100644 index 00000000..29b15388 --- /dev/null +++ b/docs/explanation/witan-core.md @@ -0,0 +1,136 @@ +# witan-core + +`witan-core` is the shared floor under both MCP servers. You never install it +deliberately — `witan-council` and `witan-code` each depend on it — but it is +worth understanding, because the things that live in it are precisely the things +that *must not differ* between the two servers. + +## Why it exists + +The two servers were built copy-paste-and-diverge, and for a while carried an +explicit convention: **deliberately duplicated — no cross-package import.** + +That held until the shared surface grew. Fixes then had to be applied twice, and +silently drifted when they weren't — including code that is *contractually* +required to stay identical: + +- The **repo-key canonicaliser**, which produces the join key linking the memory + graph to the code graph. Two implementations that disagree about whether + `git@github.com:mitodl/agent-kit.git` and + `https://github.com/mitodl/agent-kit` are the same repository do not produce a + broken build — they produce a graph where half your memories are invisible + from the other layer. +- The **pinned omnigraph binary version**, which was kept in lockstep across two + files by a fragile Renovate custom manager. A store written by one version and + read by another is a strict-format error, not a warning. + +`witan-core` reverses the duplication convention for exactly this class of code: +things where "the two copies drifted" is a correctness bug rather than an +inconsistency. + +## The invariant + +``` +witan-core ← imports neither server + ↑ ↑ +witan-council → witan-code (`witan` mounts `witan code`; never the reverse) +``` + +`witan_core` imports **neither** `witan` nor `witan_code`. It is a leaf below +both, which is what preserves the one-directional mount DAG: `witan-council` +optionally mounts `witan-code`, and `witan-code` never imports `witan-council`. + +A shared package that imported either server would turn that DAG into a cycle +and make the optional mount impossible. + +## Stdlib-only by default + +The base package has **no dependencies at all**. Everything heavier sits behind +an extra, so neither server carries weight it does not use: + +| Extra | Brings | For | +| --- | --- | --- | +| *(base)* | — | `_detach`, `repo_key`, `timeutil`, `omnigraph`, `maintenance`, `config_file`, `target_config` | +| `cli` | `cyclopts`, `rich`, `agent-config-kit` | CLI scaffolding and the installer's styled output | +| `mcp` | `fastmcp` | The `confirm`/`text` elicitation primitives | +| `remote` | `httpx2`, `fastmcp` | OIDC device auth, token cache, and the MCP-client proxy | +| `observability` | `structlog`, OpenTelemetry | Structured logs and traces | +| `sentry` | `sentry-sdk` | Error reporting, hooked onto the logging chain above | + +Two of these splits are deliberate and worth the note. `observability` is not in +the base package because a local stdio session exports nothing and an OTLP +exporter is a lot of weight for a `pip install`. `sentry` is separate from +`observability` because it hooks the stdlib logging chain that `observability` +already sets up rather than introducing a new pipeline — so it needs nothing +else, and folding it in would force the OTel weight on anyone who only wanted +error reports. + +## What's in it + +The pieces most worth knowing about: + +**`repo_key`** — `normalise` and `find_git_config`. The cross-layer repo-key +canonicaliser, carrying a golden contract test. This is the highest-stakes +module in the package: its output is the identity of a repository everywhere in +witan. + +**`omnigraph.OmnigraphClient`** — the subprocess wrapper around the pinned +`omnigraph` binary, holding the write lock, the retry/repair logic, and the +admission-cap backoff. Each server subclasses it: `witan-council` adds +`apply_schema`, `witan-code` adds branch operations and bulk `load`. + +**`target_config`** — the `[targets.]` matching logic, with priority +`match_paths` > `match_repos` > `match_hosts` > `match_orgs`. Each server keeps +its own typed target model, because they carry different override fields +(`server`/`graph`/`token` for witan-council, `code_dir` for witan-code), and +calls into this shared matcher, which is structurally typed over just the four +`match_*` lists. + +**`config_file.load_toml`** — why both CLIs read one file. `witan` and +`witan code` load the same `~/.config/witan/config.toml`, so a single +`[targets.]` block configures both at once. + +**`remote/`** — the OIDC device-authorization grant, the shared token cache, and +the MCP-client proxy. This is the client stack behind `witan login`, and the +reason logging in once covers both CLIs. + +**`observability/`** — structlog configuration plus OpenTelemetry, with the OTel +halves imported defensively so an install that wants structured logs without an +exporter still works. + +Also present: `_detach.popen_detached` (cross-platform detached subprocess +spawning, used by the throttled background `optimize`), `omnigraph_install` (the +single source of the pinned binary version), `maintenance` (the +stamp/interval/due mechanics for that throttle), `elicit`, `caching`, +`chunking`, `identity`, and `timeutil.now_iso`. + +CLI scaffolding remains local to each server, deliberately. + +## The version-floor trap + +This is the one thing that will bite you when contributing. + +The uv workspace resolves `witan-core` **by path**, so a server always imports +whatever is in your checkout. An external install resolves it **by version +range** from PyPI. Those two disagree the moment you add a `witan_core` symbol +and use it in a server without raising that server's floor: + +```toml +# mcp/servers/witan/pyproject.toml +"witan-core>=0.13,<1", # ← this number +``` + +Everything passes locally and in CI. Then `pip install witan-council` resolves a +`witan-core` that does not export the symbol, and the server cannot even import. + +**If you add a `witan_core` symbol in the same change as its caller, raise that +caller's floor in that same change.** Nothing automated catches this — the +workspace's path resolution is precisely what hides it. The comments on those +pins record how often it has already happened: five times in `witan-council`, +three in `witan-code`. + +The floors track what a package actually *imports*, not what was released +alongside it. `witan-code`'s floor was deliberately **not** raised for +witan-core's trace-context work, because nothing in `witan-code` imports +`trace_context_middleware` — raising it would have bought no behaviour and +misstated the minimum. diff --git a/docs/index.md b/docs/index.md index c8b36766..fe9bab3f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -93,7 +93,7 @@ witan-context covers three published packages, all developed in the | --- | --- | | [`witan-council`](https://pypi.org/project/witan-council/) | The memory, task, and workflow tools, plus the `witan` umbrella CLI | | [`witan-code`](https://pypi.org/project/witan-code/) | The tree-sitter code graph and cross-repo bridge; mounts as `witan code` | -| [`witan-core`](https://pypi.org/project/witan-core/) | Shared internals: the graph client, OIDC/remote transport, observability | +| [`witan-core`](https://pypi.org/project/witan-core/) | [Shared internals](explanation/witan-core.md): the graph client, OIDC/remote transport, observability | | [`ol-agent-kit`](https://pypi.org/project/ol-agent-kit/) | Meta-package that installs all of the above in one shot | Storage is [omnigraph](https://github.com/ModernRelay/omnigraph) — a local file, an diff --git a/zensical.toml b/zensical.toml index 2f745632..f1ded06f 100644 --- a/zensical.toml +++ b/zensical.toml @@ -63,6 +63,7 @@ nav = [ { "Architecture" = "explanation/architecture.md" }, { "The memory model" = "explanation/memory-model.md" }, { "Coordinating work" = "explanation/task-coordination.md" }, + { "witan-core" = "explanation/witan-core.md" }, { "Code graph" = [ { "Symbol format" = "explanation/code-graph/symbol-format.md" }, { "Symbol table" = "explanation/code-graph/symbol-table.md" }, From 6e46f0762af0afc49cbf7d6f1ca03f796a134f0d Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Wed, 19 Aug 2026 12:25:37 -0400 Subject: [PATCH 2/2] fix(docs): correct three witan-core claims, two of them copied from a stale README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback on #255 (Copilot). All three verified against the code; all three were wrong, and two came from trusting `packages/witan-core/README.md` rather than checking what the package does now. **The dependency example was a stale fragment.** The version-floor section showed `"witan-core>=0.13,<1"` as the current declaration. It is `"witan-core[cli,remote,observability,sentry]>=0.25,<1"`. Quoting an obsolete floor in the passage warning contributors about floors undercuts the warning, and could lead someone to pick an already-dead minimum. **`sentry` is additive to `observability`, not an alternative.** The page said the split lets someone take error reporting without the OTel weight. It does not: `telemetry.py` imports `witan_core.observability.logging`, which imports `structlog` at module scope, so `sentry` alone is an ImportError rather than a lighter build — and both servers request both extras anyway. The split runs the other way: a deployment can have structured logs and traces *without* shipping errors to Sentry. **CLI scaffolding is shared, not local.** The page ended by saying it remains local to each server, which is what the README still says. `witan_core.cli` provides `make_app`, `resolve_author`, and `report_install`, and both servers import them — so the sentence told contributors the opposite of the real ownership rule. What stays local is each server's own commands and setup behaviour. The README's own stale sentences are left for a separate change; correcting the package's README is not this PR's business, and the site no longer repeats it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017oVnpzjRJQVWq9hJGE2Rs7 --- docs/explanation/witan-core.md | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/docs/explanation/witan-core.md b/docs/explanation/witan-core.md index 29b15388..716f285e 100644 --- a/docs/explanation/witan-core.md +++ b/docs/explanation/witan-core.md @@ -57,13 +57,16 @@ an extra, so neither server carries weight it does not use: | `observability` | `structlog`, OpenTelemetry | Structured logs and traces | | `sentry` | `sentry-sdk` | Error reporting, hooked onto the logging chain above | -Two of these splits are deliberate and worth the note. `observability` is not in -the base package because a local stdio session exports nothing and an OTLP -exporter is a lot of weight for a `pip install`. `sentry` is separate from -`observability` because it hooks the stdlib logging chain that `observability` -already sets up rather than introducing a new pipeline — so it needs nothing -else, and folding it in would force the OTel weight on anyone who only wanted -error reports. +`observability` is not in the base package because a local stdio session exports +nothing and an OTLP exporter is a lot of weight for a `pip install`. + +`sentry` is **additive to `observability`, not an alternative to it**. It hooks +the stdlib logging chain `observability` already sets up rather than introducing +a pipeline of its own, and `telemetry.py` imports +`witan_core.observability.logging`, which imports `structlog` at module scope — +so installing `sentry` alone gets you an ImportError, not a lighter build. Both +servers request both extras. The split exists so a deployment can run structured +logs and traces *without* shipping errors to Sentry, not the other way round. ## What's in it @@ -104,7 +107,10 @@ single source of the pinned binary version), `maintenance` (the stamp/interval/due mechanics for that throttle), `elicit`, `caching`, `chunking`, `identity`, and `timeutil.now_iso`. -CLI scaffolding remains local to each server, deliberately. +CLI scaffolding is **shared**, not local: `witan_core.cli` provides `make_app`, +`resolve_author`, and `report_install`, and both servers import them. What stays +local to each server is its own commands and setup behaviour — the surface that +is genuinely different between a coordination graph and a code index. ## The version-floor trap @@ -117,7 +123,7 @@ and use it in a server without raising that server's floor: ```toml # mcp/servers/witan/pyproject.toml -"witan-core>=0.13,<1", # ← this number +"witan-core[cli,remote,observability,sentry]>=0.25,<1", # ← this floor ``` Everything passes locally and in CI. Then `pip install witan-council` resolves a