# 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_versions` use 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.Allow` against 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/callback` branch (`server.go`) to enumerate writable worlds.
- `Issuer.lookupWorld(name)` — called by `gateWrite` in `mcp_tools_write.go`. (Note: `worldWriteTokenStore` has its own private copy; could consolidate.)
- `Issuer.k8s` field — handed to `RefreshStore` and `worldWriteTokenStore` in `NewServer`.

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:

1. Per-issuance reconciliation (drift-pruning issuances Secret against world `tokens.toml`). **Dead** — issuances Secret is no longer written.
2. 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.go` and `mcp_session_test.go` entirely.
- Drop `mcpGateway.sessionCache` field and the `newSessionCache(...)` call in `mcp_gateway.go`'s constructor.
- Update `mcp_gateway.go` comments 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", ...)` from `server.go`'s Routes().
- Remove `listTokens`, `deleteToken`, `rotateToken` handlers + `listTokensResponse` type.
- 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 the `labelGen` and `maxLabelRetries` machinery if no longer reachable.
- Keep: `Issuer` struct, `NewIssuer`, `authorizedWorlds`, `lookupWorld`, `k8s` field, plus `worldAllows` / `emailMatches` / `domainMatches` / `groupsMatch` / `matchesAnyPath` / `matchPath` / `validatePattern` (still used by `authorizedWorlds`).
- 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 `Server` directly — `s.authorizedWorlds(claims)` and `s.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

1. **Sweeper option A vs B.** Lean B (delete, ticker-in-RefreshStore). Confirm before coding.
2. **Rename `Issuer`** (Slice 5). Skip for now or commit to a target name. Default: skip until someone trips on it.
3. **Soul / config knobs.** `IssuancesSecret` becomes dead config. Strip from `Config` and validation in Slice 3, or leave as ignored-with-deprecation note? Lean: strip — operators reading the config shouldn't see knobs that do nothing.
4. **MCP knobs.** `FirstMintMaxAttempts` etc. 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.


## Status — COMPLETE (verified 2026-05-31)

Unblocked by **#159** (simple auth model); cleanup landed in **#164** (commit `f9a24e9`). Every targeted production symbol is gone:

- **Slice 1 — sessionCache:** `mcp_session.go` deleted; zero grep hits for `sessionCache`/`newSessionCache`/`cachedWorldToken`/`mintFunc`.
- **Slice 2 — `/tokens` API:** routes + `listTokens`/`deleteToken`/`rotateToken` removed (only a `config.go` comment remains as a breadcrumb).
- **Slice 3 — Issuer:** `issuer.go` deleted entirely (`Mint`/`MintFiltered`/`Issuance(s)`/`List`/`Revoke`/`RotateLabel`/`labelGen`); `IssuancesSecret` knob stripped.
- **Slice 4 — sweeper trim (Option A):** `sweep()` calls only `refreshStore.Sweep`; leader election retained; doc comment updated.
- **Slice 5 — rename:** moot — the `Issuer` type was deleted rather than renamed; surviving predicates (`authorizedWorlds`/`lookupWorld`) are now free functions in `authz.go`.

Correctly kept (alive by design): `RefreshStore` lifecycle + `/revoke`, `FirstMint*` retry knobs (Open Q#4). `go build ./...` clean. Completion criterion — grep each removed identifier → zero production hits — satisfied.