Correct ADR-0004 D3: there is no witan-users Keycloak group

[blarghmatey]

What are the relevant tickets?

N/A — follow-on from ol-infrastructure mitodl/ol-infrastructure#5253, which is where the misreading surfaced. Project wp-witan-multi-user-service-deployment-dcf6ee.

Description (What does it do?)

ADR-0004 D3 tells the provisioning pipeline to walk "the Keycloak witan-users group/role membership". Read literally that is an instruction to create a Keycloak group of that name — and when the pipeline was finally built in ol-infrastructure #5253, it was read exactly that way and one was created. It has since been removed after review. The wording will keep producing that mistake, so this records why it is wrong.

witan-users is a Cedar group. ADR-0002 D1 defines it as one of three groups in witan's own policy bundles (mcp/servers/witan/policy/server.policy.yaml), holding one act-<sub> per authenticated human, with membership "templated by ol-infrastructure (Keycloak claims for witan-users)". That is the accurate contract: the Cedar group is populated from Keycloak by whatever query identifies witan's users. D3 narrowed "Keycloak claims" into a same-named Keycloak object ADR-0002 never called for. The name collision is what makes the invention look mandatory.

What the pipeline actually walks: every enabled, non-service-account user of the ol-platform-engineering realm. That realm has registration_allowed=False, no identity-provider brokering and no federation, so its membership is hand-managed and already exactly the intended audience. A group inside it would be a second gate whose failure mode is somebody joining the realm, nobody adding them to the group, and hitting D3's own fail-closed path with an error that reads like a provisioning lag.

The addendum also states two things rather than leaving them to be rediscovered:

• The trade: realm access is now witan access — no way to revoke witan while leaving jupyterhub/superset/opik intact. If that requirement appears, a Keycloak group is the right answer, but it should be added deliberately for that reason.
• The consequence any reimplementation must not lose: enumerating a realm returns each confidential client's own service account as an ordinary user, so they must be filtered (serviceAccountClientId) or every such client gets the Cedar rights a person has under witan-users.

Only D3's description of the provisioning source is corrected. The decision itself — tokens pre-provisioned out-of-band, witan looks up and never mints, fail closed on a missing actor id — is unchanged, as is the Consequences section's cross-repo contract.

Also fixes the same misnomer where it was user-facing. ActorTokenResolver's unprovisioned-actor LookupError named witan-users as the thing to go check, sending an operator after a group membership that does not exist. It now points at the two things actually worth checking: whether the account is disabled, and whether it is in the realm at all.

How can this be tested?

Docs plus one error string; no behaviour change.

• cd packages/witan-core && uv sync --group test && uv run pytest — 198 passed, 1 skipped. No test asserted on the old message (checked before changing it).
• prek run on the changed files — markdownlint, ruff format and ruff all pass.

The claim worth verifying by reading rather than running is that witan-users really is only ever a Cedar group name in this repo:

rg -n 'witan-users' mcp/servers/witan/policy/ packages/witan-core/

Every hit is a policy bundle, its README, or (now) a comment explaining that no Keycloak group of that name exists.

Additional Context

Follows the file's existing ### Addendum (date) — headline convention, appended after the 2026-07-10 Resolution rather than editing D3 in place, so the original wording and the correction both stay readable.

The witan-core CHANGELOG's Unreleased section also said "adding a user to witan-users bounces the Deployment"; adjusted to "provisioning a new user" since it has not shipped yet and was describing the same nonexistent group.

🤖 Generated with Claude Code

https://claude.ai/code/session_019nGkccjfp8dkRbEKYugz6B

[Copilot]

Pull request overview

This PR clarifies (in both docs and operator-facing messaging) that witan-users is a Cedar group name, not a Keycloak group, and records the correct Keycloak-side source of truth to prevent future misprovisioning mistakes.

Changes:

• Update ActorTokenResolver’s unprovisioned-actor LookupError to stop referring operators to a nonexistent Keycloak witan-users group.
• Add an ADR-0004 addendum documenting the original misnomer and the actual provisioning query/scope.
• Update the witan-core changelog to reflect the corrected terminology and operator guidance.

Reviewed changes

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

File	Description
packages/witan-core/witan_core/identity.py	Updates operator-facing error text and adds inline clarification about the nonexistent Keycloak witan-users group.
packages/witan-core/CHANGELOG.md	Documents the corrected error message guidance and the Cedar-vs-Keycloak naming clarification.
mcp/servers/witan/docs/adr/0004-keycloak-jwt-per-user-actor-mapping.md	Adds an addendum correcting D3’s wording and preserving the rationale/contract going forward.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[blarghmatey]

Addressed Copilot's review in 3c43862 — all 3 threads fixed and resolved, none declined.

All three flagged the same imprecision, which is worth taking seriously here given the PR is about exactly that: I'd written "every enabled human user of the realm" where the contract the addendum states is "enabled, non-service-account user". The realm does contain service accounts; the pipeline filters them rather than the realm excluding them by nature.

The LookupError message was the one with teeth — it named disabled and absent as the two non-lag causes, but a service account is enabled and present while still legitimately having no token, so for that caller it pointed the operator at the wrong two things. It now lists all three.

198 tests pass; all 8 checks green.

Unrelated note for the record: the witan-code (code graph) job failed on the first run here with test_repo_scope_route_and_fanout. That's the flake fixed by #178 (_git_context TTL leaking across test boundaries), not anything in this diff — this PR touches one error string, a changelog and an ADR. It passed on re-run and has passed on every run since.