# 2026-05-20 — Universe Onboarding PR5

Cold-started PR5 from `/plans/universe-onboarding-pr5.md`. Both open design questions confirmed:

1. **GET vs POST for /me/install**: GET. Parent-plan-locked, matches OAuth `/me`-style endpoints. Mint side-effect documented in `meInstall`'s doc comment so future readers don't expect REST-idempotent semantics.
2. **Empty authorized worlds → 200 vs 403**: 200 + `worlds: []`. The user IS authenticated; absence of installable worlds is authz-config emptiness, not auth failure. The plugin layer needs the distinction so it can surface "no worlds authorized for your identity" vs an auth-retry.

## Changes

- `tools/demarkus-broker/internal/broker/issuer.go` — `MintFiltered(ctx, claims, keep)` added. Body of `Mint`'s authorization+iteration moved here; `Mint` is now a back-compat wrapper `MintFiltered(ctx, claims, nil)`. `keep` runs AFTER `authorizedWorlds` so the predicate input is "worlds this identity is already permitted to use." All-filtered-out returns ErrNotAuthorized (same sentinel as "zero authorized worlds in the first place") so the HTTP layer maps both branches to one response shape.
- `tools/demarkus-broker/internal/broker/issuer_test.go` — six new tests around MintFiltered: nil-predicate ≡ Mint regression guard, predicate skip + Secret assertion, reject-all → ErrNotAuthorized + no-Secret-touch, partial-failure passthrough, ErrNotAuthorized-short-circuits-before-predicate (panic predicate proves the contract), predicate-sees-authorized-worlds-only.
- `tools/demarkus-broker/internal/broker/install.go` (new) — `meInstall` handler, `installResponse`/`installWorld` structs, `toInstallWorlds` joiner, `lookupPublicURL`. PublicURL filter passed to MintFiltered as `func(w *WorldConfig) bool { return w.PublicURL != "" }` so worlds without a PublicURL get no issuance record. Cache-Control + Pragma headers set BEFORE `writeJSON` so they actually land on the wire. Partial-mint shape matches `/auth/callback`'s `partialFailure` field for consumer consistency.
- `tools/demarkus-broker/internal/broker/install_test.go` (new) — 12 tests covering happy path (1 world), multi-world ordering (declaration order pinned), PublicURL-less filtered out + no Secret churn, no-bearer 401, bad-bearer 401, empty-worlds → 200+[], all-worlds-filtered → 200+[], unverified email → 403, partial mint → 200 + partialFailure flag, hard failure → 500, Cache-Control/Pragma headers, broker-signed bearer accepted (regression guard against PR4's compositeVerifier dispatch).
- `tools/demarkus-broker/internal/broker/server.go` — `mux.Handle("GET /me/install", authedSubject(s.meInstall))` registered in `Routes()`. Same composition as `/tokens` routes (requireAuth → subjectRateLimit).
- `tools/demarkus-broker/main.go` — package-doc Current-scope comment updated to mention `/me/install`.
- `deploy/helm/demarkus-broker/README.md` — new "Endpoint surface" section with a table of every broker route + auth/rate-limit/notes. `/me/install` subsection documents the response shape, the PublicURL-filter rule, the 200-empty empty-worlds policy, the partial-failure shape, and the broker-signed/IdP-signed bearer-accept story.

## Design decisions that landed differently from the plan

- **MintFiltered's `keep` runs AFTER AllowConfig, not before.** Plan diagrammed it "after authorizedWorlds and before per-world mint." Tightened in implementation: the predicate must NEVER run on an unauthorized caller (the new `TestMintFilteredRejectsUnauthorizedBeforePredicate` test pins this with a `panic()` predicate that fires loudly if the contract regresses). Reasoning: a buggy predicate could otherwise be coaxed into running against fabricated WorldConfig pointers if the unauthorized-short-circuit moved.
- **`toInstallWorlds` tolerates empty `PublicURL` lookup misses.** A world that vanishes from `cfg.Worlds` between MintFiltered's iteration and the response-build (operator hot-reload mid-request) would land in the response with `publicURL: ""`. Two options: (a) drop the row, (b) surface it with empty URL. Picked (b) — config-time race is a logging-and-metrics story; dropping a row would also need a partial-failure flag set, doubling the failure-mode surface for an edge case that doesn't happen in production wiring (cfg is loaded once, never mutated). The consumer (`tools/demarkus-join`, PR6) treats empty `publicURL` as "skip this row" so the user sees "X out of Y worlds installed."
- **`fakeVerifier` already had a `verifyFn` hook from PR4.** Was tempted to add a new helper for "verifier that errors on VerifyIDToken"; reused the existing hook instead. Pattern worth remembering for next time: the test-double surface already has the lever; check first.

## Verification

- `go test -race -count=1 ./internal/broker/` → green (~6s; +18 new tests: 6 MintFiltered + 12 meInstall)
- `helm unittest .` (broker chart) → 72/72 pass, unchanged from PR4 baseline (no chart changes — README is the only doc surface that moved)
- `bash pre-commit.sh` → format, vet, lint clean across protocol/server/client/tools

## Scope estimate vs actual

| Step | Plan code | Actual code | Plan tests | Actual tests |
| --- | ---: | ---: | ---: | ---: |
| 1. Issuer.MintFiltered | 40 | 42 | 80 | 209 |
| 2. meInstall handler | 80 | ~115 | 200 | ~365 |
| 3. Route registration | 5 | 8 | 0 | 0 |
| 4. Documentation | 30 | ~72 | 0 | 0 |
| **Total** | **~155** | **~237** | **~280** | **~574** |

Tests came in roughly 2× the estimate — the partial-failure + Cache-Control + broker-signed-bearer matrix is denser than the plan accounted for, and the install_test.go file isolates fixtures (installTestConfig, installTestConfigTwoWorlds) that the plan assumed would be shared with issuer_test.go. Worth it: every test covers a distinct invariant that would be expensive to debug in PR6/PR7 if it regressed silently.

## Next

PR5 ready to open. Fritz handles the commits (per `/patterns.md` "Fritz handles all commits himself"). Branch is `feat-tools-broker-me-install`. After review + merge:

- **PR6** (`tools/demarkus-join` binary) consumes the JSON shape PR5 emits. Wire contract: `{name, publicURL, label, accessToken, expiresAt}` per world; PR5 pins this in `install_test.go` so a future field-rename ripples through tests.
- **PR7** (plugin slash commands + kind Stage 5) drives PR6.
- **PR8** (docs) is the operator-facing universe-onboarding write-up.

## Touch points worth remembering for PR6

- The plugin layer must NOT treat `worlds: []` as an auth error. The broker returns 200; the plugin surfaces "no worlds authorized for this identity."
- A `partialFailure: "one_or_more_worlds_failed"` field is informational, not fatal — install the worlds that did come through and tell the user some are missing.
- `accessToken` is a one-time disclosure. Once it lands in `tools/demarkus-join`, write it to the world client config and discard the in-memory copy; the broker can't reissue it (raw material is unrecoverable).
- Cache-Control + Pragma headers are set by the broker, but the consumer should also avoid logging the response body — bearer tokens leak via accidental error-path stack traces.


## Broker MCP Gateway — kickoff (Pre-Flight 0 + 1)

Same-day pivot from PR5 (#141 shipped) into the gateway plan. Plan v3 read top-to-bottom; only Open Question 1 is settled, #2-#8 carry forward with their documented leans.

### Pre-Flight 0 — hoist `client/internal/fetch` + `merge` to public

Branch `feat-client-hoist-fetch-public`. Mechanical relocation:

- `client/internal/fetch/fetch.go` → `client/fetch/fetch.go`
- `client/internal/merge/{merge,diff3,merge_test,diff3_test}.go` → `client/merge/...`
- Import paths updated in 9 consumers (broader than the plan's "3 cmd dirs" — also `demarkus-agent`, `fedcrawl`, `graphstore`). The grep before the move caught this; worth noting in the plan-vs-reality column.

Verification:
- `go test -race -count=1 ./...` inside `client/` → green; same package list as baseline plus `client/fetch` (no test files) and `client/merge` (tests survive the move intact).
- `bash pre-commit.sh` → format/vet/lint clean across protocol/server/client/tools.
- `git status` shows 5 renames + 9 modified imports. Git's rename detection caught all 5 as zero-diff renames.

Why this matters for the gateway: `tools/demarkus-broker` lives in a separate Go module under monorepo `replace` directives. Go's internal-package rule would have blocked the broker from importing `client/internal/fetch` and `client/internal/merge` directly. The hoist is the unlock — broker's per-tool handlers in Slice 2/3/6 can now call `fetch.Client` and `merge.Candidate` without copy-paste.

Branch waiting on Fritz for commit + PR review (per `/patterns.md` "Fritz handles all commits himself").

### Pre-Flight 1 — Streamable HTTP support in mcp-go v0.44.0

Decision: **Streamable HTTP, via mcp-go's `server.NewStreamableHTTPServer`.** No transport swap or in-house implementation needed.

Evidence: `~/go/pkg/mod/github.com/mark3labs/mcp-go@v0.44.0/server/streamable_http.go` exports `StreamableHTTPServer` as a first-class production type (not just a test helper). It implements `http.Handler` — drop-in for whatever HTTP mux the broker's MCP listener uses, and the default URL path is `/mcp`, exactly the shape Slice 1 calls for. Three usage modes are documented (`.Start(":8080")`, `http.Handle("/path", handler)`, or `ServeHTTP` directly). Targets the [2025-03-26 transport spec](https://modelcontextprotocol.io/specification/2025-03-26/basic/transports#streamable-http).

One caveat from the package doc: "The current implementation does not support Stream Resumability." Irrelevant here — the plan's Out-of-Scope already excludes long-lived subscriptions and server-initiated events; tool calls are JSON-RPC request/response. Polling is the model.

Slice 1 shape this implies:
- `mcpGateway` wraps `server.NewStreamableHTTPServer(s.mcpServer)` and mounts it on a fresh `http.ServeMux` separate from the existing management API.
- `gatewayAuth` middleware composes around the StreamableHTTPServer (standard `http.Handler` chain): id_token verification + per-subject rate-limit happen before the handler ever sees the request body.
- OAuth metadata endpoints (`/.well-known/oauth-protected-resource`, `/.well-known/oauth-authorization-server`) register on the same mux, plain `mux.HandleFunc`.

Stdio-mode `client/cmd/demarkus-mcp` keeps using `ServeStdio` — unchanged. Two transports, one tool surface, eventually.

### Next

Branch out of Pre-Flight 0 once merged, cut `feat-tools-broker-mcp-gateway-foundation` for Slice 1.


## Broker MCP Gateway — Slice 1 (foundation: listener + initialize + tools/list)

Branch `feat-tools-broker-mcp-gateway-foundation`. Built on top of the just-merged Pre-Flight 0 hoist (`client/fetch` + `client/merge` now public).

### Files

Production (~660 LOC):

- `tools/demarkus-broker/internal/broker/config.go` (+85) — `ServerConfig.MCP` (`Enabled`, `Addr`, `TLS{CertFile, KeyFile}`); `WorldConfig.InternalAddress` for Service-DNS override (consumed in Slice 2). `MCPConfig.validate()` enforces addr-when-enabled + paired TLS fields; hook wired into outer `Config.validate()`.
- `tools/demarkus-broker/internal/broker/mcp_gateway.go` (117) — `mcpGateway` struct (wraps mcp-go `MCPServer` + `StreamableHTTPServer`). `newMCPGateway` registers the 13 tools with the Slice 1 placeholder. `Routes()` mounts `/mcp` behind `gatewayAuth → subjectRateLimit → mcp transport`, plus the two `.well-known` metadata endpoints. Exported `Server.MCPGateway(version)` returns `nil` when MCP is disabled — main.go branches off the result.
- `tools/demarkus-broker/internal/broker/mcp_auth.go` (52) — `gatewayAuth` middleware: same composite-verifier chain as `requireAuth`, but the 401 carries an RFC 6750 + RFC 9728 `WWW-Authenticate` challenge with `resource_metadata=<broker>/.well-known/oauth-protected-resource`. The 401 envelope is the only difference from `requireAuth`; rest of the auth machinery (compositeVerifier, claims-on-ctx) is shared verbatim.
- `tools/demarkus-broker/internal/broker/mcp_oauth.go` (46) — RFC 9728 protected-resource metadata handler. Static JSON document keyed off `cfg.Server.PublicURL`; the RFC 8414 authorization-server endpoint reuses Discovery's existing handler (broker is its own auth server, RFC 8414 §3 tolerates OIDC's extra fields). The OIDC issuer-override Discovery already applies makes the same body usable as RFC 8414 metadata for a broker-as-auth-server deployment.
- `tools/demarkus-broker/internal/broker/mcp_tools_list.go` (311) — 13 `mcp.Tool` builders + `mcpToolNames` list. URL-format description swapped from the local demarkus-mcp's "bare path / mark://host:port" copy to **"mark://{worldName}/{path}"** — the broker addresses worlds by name and the description must say so explicitly so the LLM picks the right shape. `mcpToolNames` and `mcpTools()` stay in declaration-order sync so a single regression test pins the surface.
- `tools/demarkus-broker/main.go` (+50) — second `http.Server` started alongside the management API when `srv.MCPGateway(version) != nil`. TLS handled inline (ListenAndServeTLS when CertFile set, else plain). Shutdown handled best-effort alongside the management-API shutdown; mcp-side errors logged but don't override the primary path.
- `tools/go.mod` (+7) + `tools/go.sum` (+16) — `github.com/mark3labs/mcp-go v0.44.0` direct dep; transitive deps pulled in by `go mod tidy`.

Tests (~798 LOC, all passing under `-race`):

- `mcp_gateway_test.go` (347) — end-to-end: disabled-by-default, initialize handshake, tools/list returns exactly the expected 13 (and ONLY tools capability — no resources/prompts), tools/call returns Slice 1 placeholder envelope (`isError:true`, message names the tool), per-subject rate-limit triggers 429 with Retry-After, cross-subject isolation (alice exhausts → bob still fresh). Helpers `mcpTestConfig` + `newTestMCPGateway` mirror existing `testConfig` + `newTestServer` shape.
- `mcp_auth_test.go` (129) — missing-bearer 401 with `realm="demarkus-broker"`, `error="missing_bearer"`, and `resource_metadata=<broker>/.well-known/oauth-protected-resource`; invalid-bearer 401 with `error="invalid_token"`; valid-bearer composes the claims onto downstream context.
- `mcp_oauth_test.go` (129) — RFC 9728 endpoint serves resource + authorization_servers + bearer_methods_supported + scopes_supported with the broker's PublicURL; Cache-Control header present (mirrors OIDC well-known); RFC 8414 alias mounted when Discovery is wired (issuer override applied); absent (404) when Discovery is nil.
- `mcp_tools_list_test.go` (119) — exact-13 surface, every URL-taking tool's description references `mark://`, required-vs-optional argument schema pins (mark_publish: expected_version required; mark_append: expected_version optional + auto-resolved in handler; mark_resolve: hash + index; mark_index: source + target).
- `config_test.go` (+74) — `MCPConfig.validate()` table: disabled-zero-valid; disabled-with-partial-fields-ignored; enabled-with-addr OK; enabled+both-TLS-fields OK; enabled-no-addr fails; enabled+cert-without-key fails (and vice versa); plus one YAML-round-trip case to pin the validation hook is actually called from outer `Config.validate()`.

### Decisions that landed differently from the plan / worth pinning

- **Gateway as Server method, not separate listener struct.** Plan called for a `mcpGateway` listener type. Implementation: `mcpGateway` is still a type (owns the mcp-go server + transport), but the listener wiring lives in main.go alongside the existing management API server. `Server.MCPGateway(version)` is the entry point — returns `nil` when MCP is disabled. Reasoning: Server already owns verifier + rate-limit + Issuer + Discovery — the gateway needs all of them. Building a parallel listener struct would have duplicated the dependency wiring. The separation the plan asked for is preserved at the listener level (different `http.Server`, different `Addr`), not the type level.
- **`Server.gatewayAuth` is a sibling of `requireAuth`, not a wrapper.** Considered composing `requireAuth → "rewrite-401-with-WWW-Authenticate"` middleware. Rejected: requireAuth writes the 401 inline (no late-stage interception), and the alternative would require restructuring the existing auth flow. Cleaner to copy the two-step verification path into `gatewayAuth` and emit the RFC 6750/9728-shaped 401 directly. Cost: ~25 lines of duplication, but the existing routes' 401 shape is intentionally minimal (no challenge header) and forcing them to grow one would break the contract /tokens callers expect.
- **`oauth-authorization-server` route aliases the Discovery handler, no second renderer.** RFC 8414 §3 tolerates OIDC's extra fields, and the OIDC discovery doc the broker already serves (with PR4's broker-as-issuer override) describes exactly the same auth server. One handler, two route registrations. Mounted on the MCP listener only — the management API doesn't need it.
- **Streamable HTTP session-ID handshake is real and tests have to thread it.** `mcp-go.NewStreamableHTTPServer` defaults to `StatelessGeneratingSessionIdManager`: server emits `Mcp-Session-Id: mcp-session-<uuid>` on initialize, validates the format on every subsequent request. Test helpers must capture the header from init and echo it on tools/list + tools/call. Hard-coded the literal `"Mcp-Session-Id"` in the test file rather than importing `mcpserver.HeaderKeySessionID` to keep test imports lean. Worth knowing for Slice 2+ test patterns.
- **gocritic's hugeParam doesn't know about mcp-go's `req mcp.CallToolRequest` (value receiver) contract.** Same `//nolint:gocritic // signature required by mcp-go` annotation as the local demarkus-mcp uses. Three real lint nits were also caught: `httptest.NewRequest(..., nil)` → `http.NoBody`, `for _, t := range mcpTools()` → indexed range (320-byte copy per iteration), and the noted hugeParam exception.

### Tool description shape

`mcpURLHint` reads: `"URL form: mark://{worldName}/{path}. {worldName} is one of the worlds in this knowledge system; the broker routes the request to the appropriate world server."` — appended to every tool's description except `mark_graph_export` (no URL argument). `mcpURLDesc` reads `"mark:// URL, e.g. mark://team-a/index.md"` for per-arg JSON-schema descriptions. The LLM sees the worldName-as-host pattern consistently; Slice 2's URL parser will validate it server-side.

### Scope estimate vs actual

| Element | Plan code | Actual code | Plan tests | Actual tests |
| --- | ---: | ---: | ---: | ---: |
| Slice 1 (foundation) | ~500 | ~660 | ~600 | ~798 |

Code modestly over (~160 LOC, dominated by the 13 tool builders being wordy; the descriptions are intentionally explicit). Tests modestly over (same pattern as PR5: each invariant pinned distinctly catches denser than the plan estimate).

### Verification

- `go test -race -count=1 ./demarkus-broker/internal/broker/` → green, ~6s
- `bash pre-commit.sh` → format/vet/lint clean across protocol/server/client/tools
- Branch awaiting Fritz for commit + PR review

### Next

Slice 2 (read tools + session cache foundation: mark_fetch, mark_list, mark_versions + `sessionCache` + `worldPool`) begins after this lands. Slice 2 imports `client/fetch` for the first cross-module consumer of the Pre-Flight 0 hoist — exercises the public boundary the hoist was specifically for.


## Broker MCP Gateway — Slice 1 revision: gateway is always on

Reviewed Slice 1 config shape with Fritz; "opt-in deployment" came off the table. Two rounds of pushback ended at the right design:

1. **First pass** had `MCPConfig{Enabled bool, Addr string, ...}` mirroring the plan's Non-Negotiable. Fritz: the Enabled flag is redundant with Addr-presence and creates contradictory states (`Enabled=false + Addr=":8081"` silently ignores the addr).
2. **Second pass** dropped Enabled, made Addr-presence the on-switch. Fritz: the universe-onboarding-only broker shape isn't a real enterprise deployment — every broker is a knowledge-system gateway. MCP is part of the binary, not a feature flag.

**Landed shape:**
- `MCPConfig{Addr string, TLS MCPTLSConfig}` — no boolean gate
- `Addr` defaults to `:8081` (`defaultMCPAddr` constant) when omitted, so existing universe-onboarding values.yaml files upgrade silently — the listener appears on the upgrade, operators tune the port in a follow-up if they need to
- `Server.MCPGateway(version)` always returns a non-nil handler; `main.go` always starts the second listener
- `MCPConfig.validate()` enforces paired-TLS-fields; no addr-required check (defaulted instead)

**Plan revision:** the plan's `Non-Negotiables` line *"Opt-in deployment. The MCP listener is off by default."* is now contradicted by the shipping code. Slice 7 (chart) needs to reflect this — every chart deployment ships with the gateway, no `enabled: false` knob in values.yaml. I'll update `/plans/broker-https-gateway.md` to remove that Non-Negotiable when I cycle back to it at end-of-session.

**Test updates:**
- `TestMCPGatewayNilWhenAddrEmpty` deleted (no longer a reachable state)
- `TestMCPConfigValidate` reorganized around default-applied + paired-TLS-fields
- New `TestLoadConfigMCPDefaultsAppliedWhenBlockOmitted` pins the silent-upgrade behavior for pre-gateway YAML configs

**Verification:** `go test -race -count=1 ./demarkus-broker/internal/broker/` green; `bash pre-commit.sh` clean.

**Lesson:** when a plan has an "opt-in" knob, question whether it's real product flexibility or plan-as-documentation. The Enabled flag was the latter — the plan said "opt-in" as a deployment posture, I encoded it as a runtime config. The right encoding for "this is part of the binary" is no knob at all. Same shape as the Sweeper's `Disabled bool` — wait, the Sweeper does have a disable switch. Different reason: leader-election requires k8s Lease and per-replica state, so a single-replica or k8s-less dev environment needs a way to skip. MCP doesn't have that constraint; the listener is just an http.Server. Keep the comparison in mind for future Slice work — disable-switches are for capabilities that have a real "off" deployment, not capabilities that are part of the product.


## Related documents

- [Universe onboarding PR5](/plans/universe-onboarding-pr5.md): the plan this session implemented
- [Broker HTTPS gateway](/plans/broker-https-gateway.md): gateway plan driving the MCP slices
- [Universe onboarding](/plans/universe-onboarding.md): parent plan for the PR5 wire contract
- [Patterns](/patterns.md): commit workflow followed for branches here