Broker — Post-Simplification Dead-Code Cleanup
Context
PR #159 (feat/broker-open-knowledge-system) replaced the broker's per-user-token model with:
- Reads dispatched unauthenticated (
mark_fetch/mark_list/mark_versionsuse empty token). - One long-lived write token per world, stored in a broker-namespace Secret, shared across all writers.
- Writer authorization at the broker via
gateWrite(WorldConfig.Allowagainst SSO claims) before any dispatch.
That left a lot of code unreachable from any live path. This plan lands AFTER #159 has been observed working in production (knowledge.demarkus.io) — we deliberately keep the dead code around through the initial rollout as a safety net.
Inventory (verified 2026-05-27 against feat/broker-open-knowledge-system HEAD)
Definitely dead (no live callers)
| Surface | Files | LOC est. |
|---|---|---|
mcp_session.go (sessionCache + session + cachedWorldToken + mintFunc) |
mcp_session.go, mcp_session_test.go |
~770 |
Issuer.Mint, MintFiltered, mintForWorld, MintResult |
issuer.go |
part of ~1200 below |
Issuer.List, Revoke, RotateLabel |
issuer.go |
⇧ |
Issuance, Issuances, IssuancesSecretKey, readIssuances, appendIssuance, removeIssuance, revokeIssuance |
issuer.go |
⇧ |
/tokens, /tokens/{label} DELETE, /tokens/{label}/rotate routes + listTokens/deleteToken/rotateToken handlers + listTokensResponse |
server.go |
~250 |
| Sweeper's issuance-reconciliation logic | sweeper.go, sweeper_test.go |
~400 (most of) |
| Tests for all of the above | issuer_test.go, sweeper_test.go, parts of server_test.go |
~80% of issuer_test.go (~1040 of 1305) |
mcpGateway.sessionCache field + newSessionCache initialization |
mcp_gateway.go |
~3 |
Still alive in slimmed form
Issuer.authorizedWorlds(claims)— called by/me/install(install.go) and the bare-code/auth/callbackbranch (server.go) to enumerate writable worlds.Issuer.lookupWorld(name)— called bygateWriteinmcp_tools_write.go. (Note:worldWriteTokenStorehas its own private copy; could consolidate.)Issuer.k8sfield — handed toRefreshStoreandworldWriteTokenStoreinNewServer.
The type name Issuer no longer fits — it doesn't issue anything. After the strip it's a thin "world registry + shared k8s handle." Rename is optional (see Open Questions).
Sweeper
sweeper.go currently runs two responsibilities in one leader-elected loop:
- Per-issuance reconciliation (drift-pruning issuances Secret against world
tokens.toml). Dead — issuances Secret is no longer written. - Refresh-token sweep (
RefreshStore). Still alive — broker's own bearer refresh tokens still expire and need cleanup.
After (1) is gone, the leader-election plumbing is overkill for (2) — refresh-token sweep can run on every pod with duplicate work being merely wasteful, not incorrect. But keeping leader election isn't wrong either; it's just disproportionate.
Slices
Each slice compiles and go test ./... passes independently.
Slice 1 — Delete sessionCache
- Remove
mcp_session.goandmcp_session_test.goentirely. - Drop
mcpGateway.sessionCachefield and thenewSessionCache(...)call inmcp_gateway.go's constructor. - Update
mcp_gateway.gocomments that reference the removed cache.
Smallest, most isolated. Self-contained.
Slice 2 — Delete /tokens HTTP surface
- Remove
mux.Handle("GET /tokens", ...),mux.Handle("DELETE /tokens/{label}", ...),mux.Handle("POST /tokens/{label}/rotate", ...)fromserver.go's Routes(). - Remove
listTokens,deleteToken,rotateTokenhandlers +listTokensResponsetype. - Remove tests for those routes from
server_test.go.
After this slice, Issuer.List / Revoke / RotateLabel have zero callers. (They're still defined in issuer.go; Slice 3 deletes them.)
Slice 3 — Strip Issuer to writer-predicate + lookup
- Delete from
issuer.go:Mint,MintFiltered,mintForWorld,MintResult,Issuance,Issuances,IssuancesSecretKey,readIssuances,appendIssuance,removeIssuance,revokeIssuance,List,Revoke,RotateLabel,ErrNotFound,ErrNotOwner, plus thelabelGenandmaxLabelRetriesmachinery if no longer reachable. - Keep:
Issuerstruct,NewIssuer,authorizedWorlds,lookupWorld,k8sfield, plusworldAllows/emailMatches/domainMatches/groupsMatch/matchesAnyPath/matchPath/validatePattern(still used byauthorizedWorlds). - Massive trim of
issuer_test.go— remove every test that exercised Mint / Revoke / RotateLabel / List / Issuance bookkeeping.
This is where the bulk of the LOC reduction lands.
Slice 4 — Trim Sweeper to refresh-only (or delete entirely)
Two options, equally valid:
Option A — Trim. Keep Sweeper struct, drop the issuance-reconciliation methods (sweep, readWorldLabels, perWorld, toRevoke). runOnce collapses to "sweep refresh tokens." Leader election retained.
Option B — Delete. Remove Sweeper entirely. Add a RefreshStore.SweepLoop(ctx) method that runs on a time.Ticker on every pod. Lose leader election (duplicate sweep work is wasted but harmless). Net less code.
Recommended Option B — it removes more code (whole Sweeper machinery + leader-election lease coordination), and the only loss is "occasionally two pods both call the same idempotent delete." But A is fine if the user wants to preserve the existing observable behavior.
Slice 5 (optional, deferrable) — Rename Issuer
The type's responsibilities are now: enumerate worlds, lookup world by name, hold a k8s client handle for other stores to borrow. Reasonable rename targets:
worldRegistry— accurate, single-responsibility.- Fold into
Serverdirectly —s.authorizedWorlds(claims)ands.lookupWorld(name). Eliminates one type.
Pure cosmetic. Diffs touch every callsite. Decide separately or skip.
Ship shape
One PR for slices 1-4, on a branch off feat/broker-open-knowledge-system (or main after #159 lands). Single coherent review: "post-simplification cleanup, all deletions are of code unreachable since #159." Reviewers can audit by grepping for each removed identifier and confirming the only callers are also being removed in the same diff.
Slice 5 (rename) ships separately. The naming-change diff is noisy and would distract from the deletion review.
Open questions
- Sweeper option A vs B. Lean B (delete, ticker-in-RefreshStore). Confirm before coding.
- Rename
Issuer(Slice 5). Skip for now or commit to a target name. Default: skip until someone trips on it. - Soul / config knobs.
IssuancesSecretbecomes dead config. Strip fromConfigand validation in Slice 3, or leave as ignored-with-deprecation note? Lean: strip — operators reading the config shouldn't see knobs that do nothing. - MCP knobs.
FirstMintMaxAttemptsetc. are still load-bearing for the one-time first-write propagation wait. Keep, with refreshed doc comments naming the actual remaining purpose.
Prerequisites
- PR #159 merged.
- Field observation on
knowledge.demarkus.io: at least one writer's first write to a freshly-provisioned world succeeds, confirming the new mint path works end-to-end under real kubelet propagation timing. - Broker uptime stable for a few days after #159 lands — gives time for any unexpected regression to surface against the dead code, before we delete the safety net.
Status
- 2026-05-27 — Plan drafted on soul, awaiting #159 merge + field bake.