# 2026-05-21

## Broker MCP Gateway — Slice 2 (read tools + session cache foundation)

Branch: `feat-tools-broker-mcp-gateway-read-tools`.

Built the read-side of the gateway: lazy-minted, per-email cached world tokens drive `mark_fetch` / `mark_list` / `mark_versions` against the worldPool. The remaining 10 tools still hit `notImplementedHandler`. Plan reference: `/plans/broker-https-gateway.md` v5.

### Decisions made this session that weren't in the plan

- **Session cache is keyed by canonical verified email.** Plan v4 was subject-hash-keyed; investigated the existing Issuer.MintFiltered and found the broker already canonicalizes email and uses it as the ownership key across `/auth/callback`, `/me/install`, `/tokens` list/revoke, and audit logs. Two identity dimensions in one broker would have been confusing. Published v5 of the plan pinning email-keyed session cache as a Non-Negotiable, and updated the slice accordingly. Subject claim stays in id_token for log correlation but is not the cache key.
- **OQ#9 resolution: retry-on-401-after-mint with exponential backoff.** The kubelet → world Secret propagation lag means a freshly-minted token can land at the world before the projected-volume refresh has run; the world then 401s. Considered three options:
  - Sync wait via a world-side `/health/tokens?hash=...` endpoint → rejected (violates "no demarkus-server changes" Non-Negotiable).
  - Prewarm at MCP `initialize` → rejected (front-loads SIGHUPs for worlds the user may never touch).
  - **Retry-on-401 with exponential backoff (chosen)** — broker-only, narrow blast radius, matches `mutateSecret`'s optimistic-concurrency posture.

  Defaults: 6 attempts, 250ms → 8s exponential, ~16s of total waiting across attempts. Configurable via `cfg.Server.MCP.FirstMintMax*`. Distinguishes "fresh mint" 401s (retry with backoff — propagation race) from "cache hit" 401s (invalidate + immediately re-mint without sleeping — operator hand-revoked, sweeper retired, etc.).

- **Singleflight on `(canonicalEmail, worldName)` mints.** Concurrent first-call bursts for the same identity+world coalesce to one Issuer round-trip. Mitigates the "mint storm" risk on broker restart (every active plugin reconnects and re-mints simultaneously). `golang.org/x/sync/singleflight` added as a direct dep on `tools/`.
- **`MCPGatewayWith(version, dispatcher)` test seam.** Production `MCPGateway()` builds a real `*worldPool` (and registers it on `Server.mcpPool` so `CloseMCPGateway()` drains pooled QUIC connections during shutdown). Tests pass a `fakeDispatcher` to drive handlers without standing up a real QUIC server. Adds `Server.CloseMCPGateway()` which `main.go` calls after `mcpSrv.Shutdown` so in-flight tool calls finish before connections close.
- **`formatToolResult` duplicated (not hoisted) from `client/cmd/demarkus-mcp/formatResult`.** 20-line helper, stable. A `formatResultReference` copy in `mcp_tools_read_test.go` is asserted byte-equal to `formatToolResult` for several `fetch.Result` cases — that's the proxy-fidelity gate. If the local helper drifts, the parity test breaks before shipping; that's when hoisting to `client/mcpfmt` becomes the right call.
- **Tool URL shape rejects triple-slash (`mark:///foo`).** `parseToolURL` only accepts the canonical `mark://{worldName}/{path}` form. Considered fallback-parsing `mark:/team-a/foo` (single-slash, world-in-path) but rejected — silently re-interpreting the first path segment as the worldName would mask agent typos. Strict shape, clear error.
- **`tools/` module now `replace`s `client/`.** Pre-Flight 0 already hoisted `client/fetch` and `client/merge` to public, but the broker module didn't have the replace directive in place because Slice 1 didn't yet import client packages. Added in this slice; `go mod tidy` then promoted `client` and `x/sync` to direct deps.

### Bugs found during testing

- **Session-cache clock vs issuer clock.** `newIssuer(t, ...)` pins the issuer's clock to 2026-05-11 (so tokens expire 2026-05-12). The gateway's session cache uses `Server.clock` which defaults to `time.Now`. Result: every minted token's `expiresAt` was already in the past by the broker's clock, so the cache treated every entry as instantly expired. Fix in `newGatewayWithDispatcher`: pin `srv.clock` to the same date as `newIssuer`. Two tests (`TestHandleMarkFetchCacheHitReusesToken`, `TestDispatchReadCacheHit401InvalidatesAndReMints`) caught it; both pass after the clock alignment.
- **Singleflight test deadlock.** Initial design used `select { case ready <- struct{}{}: default: }` to signal the test the mint was in flight, but with select-default the very first sender would lose the signal if the main test wasn't yet reading. Main then blocked forever on `<-ready`. Fix: buffered `ready` channel (size 1). Pinned in the test comment.
- **LRU eviction test using `GetOrMint` as a probe was self-defeating.** Each "is this session still cached?" check via `GetOrMint` either re-minted (mutating LRU) or moved the session to the LRU front, perturbing the very state under inspection. Switched to a direct read of `c.sessions` under the mutex.

### Scope outcome vs plan estimate

- Production code: ~1100 LOC (plan estimate: ~400). Overshoot mostly in the retry loop + mintFuncFor closure + formatter duplication + worldPool dispatcher seam.
- Test code: ~1224 LOC (plan estimate: ~500). Many edge cases — parseToolURL, propagation-race retry exhaustion, cache-hit 401 invalidation, byte-for-byte parity. Worth it.
- `go test -race ./...` green across all modules. `pre-commit.sh` (fmt + vet + golangci-lint across protocol/server/client/tools) green.

### Next session — Slice 3 starting point

- `feat-tools-broker-mcp-gateway-write-tools` branch.
- `mark_publish` / `mark_append` / `mark_archive` handlers in `mcp_tools_write.go`. Reuse `sessionCache.GetOrMint` + `worldDispatcher` (extend interface with Publish/Append/Archive methods on `worldPool`).
- Conflict envelope follows local server's shape (`expected_version`, `on_conflict: "fail"` for Slice 3 — merge-candidate is Slice 6).
- World-side RBAC failure (`not-permitted`) surfaces as MCP tool error without retry (unlike `unauthorized` which triggers the propagation-race retry loop).


## Broker MCP Gateway — Slice 3 (write tools)

Branch: `feat-tools-broker-mcp-gateway-write-tools`. Slice 2 (PR #146) merged onto main as `7c529d4`.

Lit up the three write verbs (`mark_publish`, `mark_append`, `mark_archive`). All 6 of the 13 tools are real handlers now; the 7 federation tools still hit `notImplementedHandler` until Slices 4–5.

### Decisions made this session that weren't in the plan

- **Refactored `dispatchRead` → `dispatchWithAuth(ctx, claims, worldName, op worldOp)`.** Closure-based: handlers capture per-verb args (path, body, expected_version, meta) in the closure; the retry loop only knows about token. The `readOp` enum + `dispatchOp` switch are gone. This was load-bearing for Slice 3 — without the refactor, writes would need their own retry loop and the cache-hit-401 + propagation-race semantics could silently diverge. Slice 4–5 federation tools will share the same dispatch path.
- **`on_conflict="merge"` rejected with explicit Slice 6 pointer.** Plan says "Slice 3 ships on_conflict=fail only; merge-candidate is Slice 6." The cleanest agent UX is: reject `merge` outright with a tool-error message that names Slice 6. Silently treating `merge` as `fail` would be a bait-and-switch — the agent thinks merge is being attempted when it isn't, and the world's response shape wouldn't carry the merge-candidate body the agent's prompt expects. **Default flipped to `fail`** for Slice 3; will flip back to `merge` when Slice 6 lands the candidate flow.
- **Publisher metadata derives `agent` from the canonical email, not the MCP session's client info.** The local demarkus-mcp uses `mcpserver.ClientSessionFromContext(ctx)` to get the MCP client name (e.g. "claude-code"). The broker doesn't have session-level client info on the tool-call context (the MCP layer's session info is one transport up). Using `canonicalEmail(claims.Email)` instead matches the broker's existing audit log identity dimension — so a write's `agent` field correlates with the broker's `/tokens` and `/me/install` audit entries. Operators reading world-side audit logs see a real identity instead of a generic "broker."
- **Append auto-resolve via `dispatchWithAuth(VERSIONS) → APPEND` runs both calls through the same dispatch wrapper.** Each call gets independent retry-on-401 budget. The singleflight + sessionCache from Slice 2 ensure the VERSIONS call and the APPEND call share whatever's already cached — typical case: one mint, two dispatches.
- **Compile-time guard added: `var _ mcpserver.ToolHandlerFunc = (*mcpGateway)(nil).handleMarkPublish`** (and append + archive). Catches a signature regression at build time instead of at AddTool registration time, where it would be a runtime nil-handler. Three lines, zero cost; same pattern would be cheap to extend across all 13 tools in a follow-up.
- **Conflict + not-permitted forward verbatim, not as tool errors.** Slice 2 surfaced `unauthorized` as a retry signal AND eventually as a tool error if retries exhausted. For writes, `conflict` and `not-permitted` are first-class world responses the agent acts on (re-fetch and retry, ask the operator for more scope, etc.). The plan's byte-for-byte proxy contract says forward them verbatim — that means `isError: false` on the MCP envelope; the "status: conflict" line in the formatted text is the signal. Tests pin this for both conflict (publish with version mismatch) and not-permitted (write op against a read-only token).

### Scope outcome vs plan estimate

- Production code: ~250 LOC new (mcp_tools_write.go) + ~140 LOC modified (dispatchWithAuth refactor, worldDispatcher interface extension, worldPool method additions, gateway wiring, gateway test pivot to mark_discover). **Total prod ~390 LOC vs plan ~250.** On the high side but the refactor was load-bearing.
- Test code: ~543 LOC new (mcp_tools_write_test.go) + ~60 LOC modified (fakeDispatcher extended with publish/append/archive scripted fns + writeCall record type). **Total tests ~600 LOC vs plan ~400.** Many edge cases — explicit version, auto-resolve, missing body/version, conflict pass-through, not-permitted pass-through, transport failure, on_conflict=merge rejection, on_conflict=unknown rejection, retry-inheritance, end-to-end via Streamable HTTP.
- `go test -race ./...` green across all 4 modules. `pre-commit.sh` (fmt + vet + golangci-lint × 4) green.

### Next session — Slice 4 starting point

- `feat-tools-broker-mcp-gateway-federation-read` branch.
- `mark_discover` / `mark_resolve` / `mark_backlinks` / `mark_graph` handlers in `mcp_tools_federation.go`.
- Federation tools delegate to whichever client library functions back the local MCP server's federation surface. Per the plan: "second-pass spike at start of Slice 4" to figure out the public API shape — Pre-Flight 0 hoisted `client/fetch` and `client/merge`, but federation may need additional hoists (`client/internal/index`, `client/internal/graphstore`, `client/internal/graph`, `client/internal/links`). Worth doing the spike before coding.
- These tools are read-only and don't change the session-cache shape; they reuse `dispatchWithAuth` for any tool that requires a world token, and pure local operations (e.g. `mark_backlinks` against the broker's own graph store) skip dispatch entirely.


## Broker MCP Gateway — Slice 4a (federation reads: discover + resolve)

Branch: `feat-tools-broker-mcp-gateway-federation-discover-resolve`. Slice 3 (PR #147) merged onto main as `5387701`.

Lit up the two federation read tools that don't need a broker-side graph store: `mark_discover` (well-known manifest fetch) and `mark_resolve` (hash-indexed content lookup). All 8 of 13 tools are now real handlers; the 5 placeholders are `mark_backlinks` + `mark_graph` (Slice 4b — graph-store infrastructure deferred) and `mark_index` + `mark_graph_export` + `mark_graph_publish` (Slice 5).

### Decisions made this session that weren't in the plan

- **Split Slice 4 into 4a and 4b.** Plan v5 grouped all four federation reads (discover, resolve, backlinks, graph) in one slice with a ~200 LOC estimate. The spike (task #16) found that backlinks + graph each need a broker-side graph store, plus hoists of `client/internal/graphstore` (~579 LOC) + `client/internal/graph` + `client/internal/links` (~700 LOC of new public surface). That doesn't fit a single slice cleanly, and the broker-side graph store is itself a design question (persistent vs ephemeral vs defer-to-local-demarkus-mcp) that deserves its own slice's worth of attention. Discover + resolve are clean byte-for-byte proxies; they ship in 4a per the plan's stated scope estimate. Recorded the split decision for the plan's Implementation Status section.
- **Cross-org candidate skip semantics.** A `mark_resolve` index entry pointing at a server URL the broker has no WorldConfig for surfaces from `Issuer.MintFiltered` as `ErrNotAuthorized` (not `errWorldNotFound` — `errWorldNotFound` only fires inside `worldPool.clientFor`, which is downstream of the mint). Both errors collapse to "this broker can't reach this candidate for me" from the agent's perspective, so `resolveCandidate` treats `errors.As(_, *errWorldNotFound) || errors.Is(_, ErrNotAuthorized)` as a single skippable reason class. Different internal causes (world unknown vs. world known but identity unauthorized), same recovery: try the next candidate. The skip message names the server URL so a debugging operator can correlate.
- **mark_discover requires `url` (broker has no default-host fallback).** The local `demarkus-mcp` has an optional `url` param because it can be pinned to a single host via `-host`. The broker addresses worlds by name and has no equivalent default — every tool call carries the worldName. Broker's mark_discover tool definition already required url (Slice 1); the handler matches.
- **`indexBody` helper in tests builds index documents inline.** Could have called `client/index.Build` directly, but `Build` adds a Source/Indexed/Documents header that the tests don't care about and that would couple test fixtures to the index package's metadata schema. The 5-line inline table builder is cleaner and decouples test intent from index format evolution.

### Bugs found during testing

- **`TestHandleMarkResolveCrossOrgCandidateSurfacesAsSkippedReason` initially failed** because I expected the cross-org dispatch to hit my fake (which returns `errWorldNotFound`) but the mint step ran first and returned `ErrNotAuthorized` before the dispatcher was ever called. Real fix in production code (resolveCandidate now handles both errors), not just the test. Worth pinning the test against the actual code path the production worldDispatcher would produce; not "patching the test to match the bug."
- **golangci-lint gocritic `unnamedResult`** on `resolveCandidate`'s two-string return. Named the results `(result fetch.Result, skipReason string)` so the call site reads obviously.

### Scope outcome vs plan estimate

- Pre-Flight (client/internal/index → client/index hoist): 2 file moves + 2 import updates (5 LOC of edits total). Smaller than Pre-Flight 0's fetch/merge hoist because the index package has only two consumers.
- Production code: ~190 LOC new (mcp_tools_federation.go) + ~17 LOC modified (mcp_gateway.go toolHandlers + gateway_test placeholder canary pivot to mark_backlinks). **Total prod ~207 LOC, matches the plan's ~200 LOC estimate for the original Slice 4.** Splitting off backlinks+graph kept the slice tight.
- Test code: ~470 LOC new (mcp_tools_federation_test.go). **Vs plan ~300 LOC.** Heavier than estimated because the resolve verb has many edge cases: invalid hash, missing index, index-not-found, index-non-ok, hash-not-in-index, content-hash mismatch (multi-candidate skip), cross-org skip, all-candidates-fail (last-failure naming), auth-retry inheritance. Each test pins one explicit semantic.
- `go test -race ./...` green across all 4 modules. `pre-commit.sh` (fmt + vet + golangci-lint × 4) green.

### Next session — Slice 4b OR Slice 5 starting point

Two paths forward; the design question on Slice 4b is real and worth deciding deliberately:

**Slice 4b — mark_backlinks + mark_graph:** needs a broker-side graph store. Three options:
1. **Persistent**: filesystem-backed store under broker's PVC. Heaviest — chart values, RBAC, lifecycle. Closest parity with local demarkus-mcp.
2. **Ephemeral in-memory**: lives in process, drops on restart. Lightweight; user must re-crawl after broker bounces. Acceptable for read-mostly federation views.
3. **Defer to local demarkus-mcp**: broker returns "graph functionality requires the local demarkus-mcp; install it via /soul-init" tool error. Cheapest but breaks the "13-tool parity" Non-Negotiable.

Recommendation: lead with **option 2 (ephemeral)** for Slice 4b — fits the broker's "wire-shape adapter" framing better than persistent state, keeps parity intent intact, and a persistent store is an easy follow-up if real customers ask. Worth getting Fritz's sign-off before coding.

**Slice 5 — mark_index + mark_graph_export + mark_graph_publish:** mark_index is independent of the graph store (it crawls + builds an index document via `index.Build`, then publishes to a target). mark_graph_export + mark_graph_publish both read from the graph store, so they're really Slice 4b territory.

Honest recommendation: **decide 4b's graph-store path first, then ship 4b + 5 together** — they share the graph-store dependency and a unified slice keeps the broker-side state-management story in one PR. Skipping 4b and shipping just mark_index in Slice 5 is possible but leaves the graph tools dangling for an extra cycle.


## Broker MCP Gateway — Slice 4b+5 (graph-store federation tools)

Branch: `feat-tools-broker-mcp-gateway-federation-graph`. Slice 4a (PR #148, mark_discover + mark_resolve) merged onto main as `115a09b`.

Closed out the final 5 placeholder tools in one slice: `mark_backlinks`, `mark_graph`, `mark_index`, `mark_graph_export`, `mark_graph_publish`. **All 13 tools are now real handlers.** `notImplementedHandler` remains as the defensive fallback but no advertised tool falls through to it.

### Decisions made this session that weren't in the plan

- **Ephemeral, per-pod-lifetime graph store.** Fritz signed off after the gap framing: the local `demarkus-mcp` persists its graph to `~/.mark/graph.json`, so brokered users get a semantic-parity gap (re-crawl after each broker restart). Accepted because (a) the broker's framing as a "wire-shape adapter, not stateful protocol surface" argues against persistent state, (b) pod uptime >> typical agent conversation length, (c) the gap is recoverable via `mark_graph`. Wrote `/thoughts.md` "On Bucket Stores as a k8s-Native Alternate Filestore" capturing the future direction Fritz raised: object-storage-backed persistence as a less-complicated-for-k8s alternative to PVC-backed filesystems. Parked for the post-broker design window.
- **`graphstore.New()` added as a clean in-memory constructor.** The existing `graphstore.Load(path)` required a non-empty path. New() returns an empty in-memory Store with `path=""`; `Save()` is now a no-op when path is empty so `CrawlAndPersist` can call `Save` unconditionally without the in-memory store crashing on disk write. ~10 LOC of additive change to the client/graphstore package.
- **`brokerCrawlParseURL` is lenient about queries + fragments.** Strict `parseToolURL` (which rejects them as agent-typo signals) is wrong for the crawler — links inside crawled docs legitimately carry `#section` or `?rev=2`. The crawler-only parser strips them silently so the document still gets fetched and graphed. Pinned in tests.
- **The `mark_backlinks` tool description now flags ephemerality up front** so an agent reading `tools/list` knows the property without having to trip the empty-store hint path first. Two sentences; the runtime hint still surfaces it on empty queries.
- **The placeholder canary test evolved into a static "every advertised tool has a real handler" check.** All 13 tools real means no fall-through to `notImplementedHandler`; the new test pins both directions (every name in `mcpToolNames` has a handler entry, and every handler entry is in `mcpToolNames`). A future tool def added without a handler — or a handler added without a def — fails this test instead of regressing at runtime.

### Hoist scope (Pre-Flight)

Three packages hoisted from `client/internal/` → `client/`:

- `client/graphstore` (~579 LOC across `store.go` + `export.go` + tests)
- `client/graph` (~480 LOC across `graph.go` + `crawl.go` + `adapter.go` + tests)
- `client/links` (~155 LOC)

Same shape as the Slice 2 + 4a hoists: file moves via `git mv`, sweep-updated imports across all 11 consumers (CLI, TUI, fedcrawl, mcp, and the just-hoisted packages themselves where they cross-import). Broker's `tools/go.mod` picked up `goldmark` as an indirect dep via `go mod tidy`. No semantic changes — the packages are byte-identical, just newly importable from outside the client module.

### Scope outcome vs plan estimate

- Production code: ~463 LOC new (`mcp_tools_graph.go`) + ~50 LOC modified (gateway wiring + tool description + go.mod). Plan estimate was ~200 LOC for Slice 4 + ~400 LOC for Slice 5; my 4b+5 combined hits ~500 LOC, comfortably under the original sum.
- Test code: ~656 LOC new (`mcp_tools_graph_test.go`). Many edge cases — empty store hint, backlinks-after-crawl integration, ephemeral-restart property, graph depth clamping, export-empty / export-after-crawl, publish through dispatchWithAuth, index happy path / manifest block / force override / dry run / negative expected_version, end-to-end via Streamable HTTP, broker crawl parser leniency on fragments + queries, broker crawl parser rejection of wrong schemes.
- `go test -race ./...` green across all 4 modules. `pre-commit.sh` green.

### Status after this slice

- **Slice 6** (conflict-aware merge in `mark_publish`): reuses `client/merge` (already hoisted in Pre-Flight 0). Wire the `on_conflict="merge"` branch — currently rejected with a Slice 6 pointer. Default can flip back to `"merge"` matching the local demarkus-mcp.
- **Slice 7** (chart, RBAC, docs): the broker chart needs `server.mcp.addr`, TLS, sessionMaxIdle, worldTokenTTL, worldPool, ingress + networkpolicy templates. README under `deploy/helm/demarkus-broker/`. Operator-facing MCP-API doc. The ephemeral graph-store property needs prominent placement in the chart README so operators know to expect re-crawl after restart.
- **Slice 8** (`/knowledge-join` plugin slash command): closes out the plan. Small shell-script slice in `plugins/claude-code/`.

The end-to-end "Done When" criteria from the plan now needs to happen against a real kind harness — sanity testing the broker MCP gateway with a real demarkus-server world. That's Slice 7 territory; the chart changes are the unlock.

### Next session — recommended starting point

**Slice 6** is the right next step — it's small (~150 LOC + 250 tests per plan), unblocks the on_conflict="merge" surface that's currently throwing tool errors, and lets us flip the broker's mark_publish default back to "merge" matching the local demarkus-mcp. After Slice 6 ships, the broker's 13-tool surface has full semantic parity with the local server (modulo the documented ephemeral graph-store gap).