2026-05-22

Broker MCP Gateway — Slice 6 (conflict-aware merge in mark_publish)

Branch: feat-tools-broker-mcp-gateway-merge. Slice 4b+5 (PR #149) merged onto main as fa9f86d.

Lit up the on_conflict="merge" candidate flow in mark_publish. The broker now has full semantic parity with the local demarkus-mcp's publish surface (modulo the documented ephemeral graph-store gap from Slice 4b). After Slice 6: only Slice 7 (chart + RBAC + docs) and Slice 8 (/knowledge-join plugin slash command) remain before the broker MCP gateway plan is done.

Decisions made this session that weren't in the plan

• brokerMergeAdapter captures ctx in the struct. Normally a code smell, but merge.Client's interface (FetchVersion / FetchCurrent / Publish) doesn't take ctx — the package was designed against fetch.Client which also doesn't take ctx. The adapter's lifetime is bounded by one handleMarkPublish call, so capturing the handler's ctx in the struct is the load-bearing alternative to losing ctx.Done propagation across the 3-step orchestration. Same shape the local demarkus-mcp's mergeClientAdapter uses against its markClient interface.
• Default on_conflict flipped to "merge" to match local demarkus-mcp. Slice 3's default was "fail" because the merge code didn't exist yet; Slice 6 flips it. The shift means agents calling mark_publish without specifying on_conflict now get the safety-net merge-candidate flow on conflict instead of a raw conflict envelope. Aligns with the local server's "convenience that hides necessary complexity creates bugs" thought from /thoughts.md — merge as default is the safer footgun-free choice.
• optionalIntMeta (vs the local optionalInt) to avoid name collision. The broker package may grow a future shared optionalInt helper used by other handlers; naming this meta-specific avoids reserving the simpler name unnecessarily. Same behavior as the local server's helper.
• formatMergeOutcome is duplicated, not hoisted. Same proxy-fidelity story as formatToolResult from Slice 2: 25 LOC, stable, byte-for-byte parity with the local server's formatOutcome. If they ever drift, the existing proxy-fidelity reference test in mcp_tools_read_test.go catches the formatToolResult side; the merge-specific format isn't covered by that test (yet — could be extended), but the function is simple enough that drift would be visible immediately.

Real bugs surfaced during testing

• Append + auto-resolve happy-path test had "server-version": "1.0" in fixture data. Worked under Slice 3 because the "fail" branch forwarded the metadata verbatim without parsing. Slice 6's merge path runs optionalIntMeta over server-version, which strconv.Atoi-rejects floats. Fixed the fixture to "1" (matches actual server output). The other append-side tests using "1.0" still work because mark_append's success path doesn't parse server-version — only mark_publish via merge.
• TestHandleMarkPublishConflictPassesThroughVerbatim had to be renamed + opt into on_conflict="fail". Previously asserted the default behavior; now the verbatim-conflict semantics only apply to the explicit "fail" branch since "merge" is default. Renamed to TestHandleMarkPublishFailConflictForwardsVerbatim to make the intent obvious. The test still exists and pins the still-supported opt-out behavior.
• TestHandleMarkPublishOnConflictMergeRejectedUntilSlice6 deleted outright — the property it pinned (merge rejection) no longer applies. Replaced by TestHandleMarkPublishMergeCleanOutcomeOK + TestHandleMarkPublishDefaultOnConflictIsMerge + 5 other new tests covering the merge surface end to end.
• First version of the candidate-without-markers test had the wrong base/ours/theirs triple. Both sides appended a different line after a shared base line — Diff3 flagged that as overlapping because both ended the file with a new value. Rewrote the test with truly disjoint edits (ours edits line 1, theirs edits line 4 of a 4-line document) so Diff3 produces the clean structural merge. Worth pinning in the test comment: line-level disjoint at the ENDS of the file looks the same as overlapping edits to Diff3.

Scope outcome vs plan estimate

• Production code: +163 LOC in mcp_tools_write.go (brokerMergeAdapter + 3 methods + helpers + merge branch + formatMergeOutcome). Plan estimate ~150 LOC. ✅
• Test code: +375 LOC in mcp_tools_write_test.go (3 deleted/replaced, 7 new). Plan estimate ~250 LOC. Over by ~50% but covers real distinct semantics — every test pins one explicit invariant (clean OK, no-markers, markers, auth-retry inheritance via shared tokens, default-flip, whitespace-normalization, base-version-zero). Worth it.
• go test -race ./... green across all 4 modules. pre-commit.sh (fmt + vet + golangci-lint × 4) green.

Surface status after Slice 6

All 13 broker MCP tools have full semantic parity with the local demarkus-mcp:

• 6 verbs (fetch/list/versions/publish/append/archive) with proxy fidelity + auth-race retry.
• 2 federation reads (discover/resolve).
• 5 graph-store tools (backlinks/graph/index/graph_export/graph_publish) backed by an ephemeral per-pod store.

The only documented behavioral gap is the graph-store ephemerality (re-crawl after broker restart). Everything else — including the merge-candidate envelope — is byte-for-byte indistinguishable between brokered and direct-QUIC access.

Next session — Slice 7 starting point

Slice 7 is the chart, RBAC, docs work. Per the plan:

• deploy/helm/demarkus-broker/values.yaml — server.mcp.addr, server.mcp.tls.existingSecretRef, server.mcp.sessionMaxIdle, server.mcp.worldTokenTTL, server.mcp.worldPool. worlds[].internalAddress for non-default Service DNS overrides.
• Chart templates: deployment.yaml (new containerPort + TLS mount), service.yaml (gateway port), ingress.yaml (route mcp host to new port), networkpolicy.yaml (allow ingress on mcp port).
• deploy/helm/demarkus-broker/README.md — "MCP gateway" section. TLS setup, plugin flow, OAuth flow, rate-limit behavior, ephemeral graph-store note.
• tools/demarkus-broker/MCP-API.md — operator/developer-facing spec for the 13-tool MCP surface.
• Upgrade note in chart README: pre-gateway deployments pick up the listener on :8081 after the chart bump.

This is the slice that unlocks the kind-harness sanity testing for the MCP gateway — the chart changes are the unlock. Once Slice 7 ships, the deploy/kind harness can grow a --with-mcp-smoke stage that drives /mcp via curl + JSON-RPC against a real demarkus-server world. That's the end-to-end "Done When" criterion from the plan.

Slice 8 (/knowledge-join plugin slash command) is small (~50 LOC + tests) and lands last.

Broker MCP Gateway — Slice 7 (chart + RBAC + docs)

Branch: feat-deploy-broker-mcp-gateway-chart. Slice 6 (PR #150) merged onto main as 80a4008.

Slice 7 catches the chart up to the gateway code that landed across Slices 1-6. No production-code changes — the broker binary already had MCPConfig (Addr, TLS, SessionMaxIdle, MaxSessions, FirstMint*) and WorldConfig.InternalAddress from earlier slices. This slice plumbs all of that through values.yaml, the rendered config Secret, the Deployment / Service / NetworkPolicy / Ingress templates, plus a new MCP gateway section in the chart README and a new tools/demarkus-broker/MCP-API.md operator/dev reference.

Decisions made this session that weren't in the plan

• Q1 — Ingress topology: separate hostname. Plan said "route MCP host/path to the new port" without nailing whether to share the existing host with path-routing or split into two hosts. Chose split: ingress.mcp.host is a parallel knob to ingress.host, and the chart emits a single Ingress resource with two rules + (optionally) two TLS blocks. Reason: the management API and the MCP gateway each have their OWN .well-known/* surfaces — OIDC discovery on the management side (openid-configuration, jwks.json) and OAuth metadata on the MCP side (oauth-protected-resource, oauth-authorization-server). Same-host path-routing would create a fragile precedence ladder the first time either side grew a new .well-known/* endpoint. Two hostnames = zero collision risk + independent cert rotation. The OCI 8414 metadata aliasing the OIDC Discovery handler (per Slice 1's plan note) is a same-listener choice; the ingress topology is orthogonal.

• Q2 — Chart version bump: cosmetic only. Chart.yaml bumped 0.1.0 → 0.2.0 (version + appVersion in lockstep). Discovered the release pipeline at .github/workflows/release.yml:520 overrides both fields at package time with ${{ needs.semver-tools.outputs.new_version }} — the in-tree value is documentation, not load-bearing. Latest published tools/v* tag is tools/v0.1.15, so the kind harness BROKER_CHART_VERSION got the matching bump (0.1.3 → 0.1.15). The 0.2.0 in Chart.yaml signals "Slice 7 ships substantive chart support" but the next published version will be whatever conventional-commits gives — probably 0.1.16, not 0.2.0. Worth flagging when reviewing: ignore the in-tree number.

• Q3 — Broker-side TLS supported but Ingress-terminated is recommended. server.mcp.tls.existingSecretRef.name is the chart's ONE TLS mode — pointing at a pre-existing kubernetes.io/tls Secret. Chart mounts read-only at /etc/demarkus-broker/tls/mcp/ and renders certFile/keyFile paths into the broker's config. No in-line PEM mode (cleartext private keys in helm release history are never acceptable). README documents Ingress-terminated as the default + recommended path; broker-terminated is for mTLS broker↔Ingress topologies or Ingress-bypass deployments.

• Q4 — --with-mcp-smoke ships in Slice 7 but scoped down. Originally proposed the full id_token + initialize + tools/call flow. Scoped down to three lightweight checks: RFC 9728 metadata fetch, RFC 8414 metadata fetch, POST /mcp without auth → 401 + WWW-Authenticate. Reason: the full id_token dance requires device-flow + refresh-grant orchestration in shell, which is itself ~100 LOC and is the natural test surface for Slice 8's /knowledge-join slash command. The three checks prove what Slice 7 actually needs to prove — the chart's MCP listener binds, OAuth metadata renders, auth gate fires. Anything more is testing the binary, not the chart.

• worldTokenTTL knob from the plan is NOT in values.yaml because the broker binary doesn't actually have a WorldTokenTTL field — the only mention is a doc-comment on MCPConfig. Slice 2's implementation went with "natural expiry from Issuer.MintFiltered" (plan OQ#5 lean answer) and never plumbed the override. Adding a chart knob the broker silently ignores is exactly the "Question opt-in knobs from plans" footgun from auto-memory. Replaced with a values.yaml comment pointing operators at worlds[].defaultToken.expiresAfter instead. Plan vs code drift, captured here.

• mcpPort helper extracts the port from server.mcp.addr so Deployment / Service / NetworkPolicy / config-render all use one source of truth. The alternative — surfacing both addr (string) and port (number) as separate values — would invite drift. The helper fails template render with a clear message if addr lacks a parseable port, better than rendering containerPort: 0 and crashing on bind.

• Smoke checks deferred to Slice 8 are listed in code comments so the next person doesn't replicate scope expansion. The kind smoke proves chart-wiring; full E2E proves binary semantics — different proofs, different slices.

Plan vs implementation drift discovered

• WorldTokenTTL is a plan field that never landed (above).
• The plan also listed server.mcp.worldPool for Slice 7's values surface. There is no MCPConfig.WorldPool substruct in the binary — the worldPool's lifecycle is internal to Server.MCPGateway(), not configurable. Dropped from values.yaml on the same "no knob for a missing field" principle.

Bugs found during testing

• Initial cert-manager Certificate test asserted containsDocument against the same template that already renders two ----separated docs. helm-unittest's containsDocument is per-document-index, not file-wide. Two assertions targeting documentIndex 0 and 1 of the same render kept failing in confusing ways. Split into separate it: cases: one asserts hasDocuments: count 2 when both hosts are cert-manager, another asserts the full Certificate shape against an MCP-only render (single document, default index). Cleaner test intent, no documentIndex juggling.
• The set: server.mcp.addr: "" test case was unreachable — helm-unittest treats empty-string set values as "no override," so the values.yaml default :8081 always won. The fail-fast guard for blank addr is still real (verified manually via helm template --set server.mcp.addr=), just not unit-testable via helm-unittest. Removed the assert; the manual verification + the in-helper fail message are sufficient defense.
• server.mcp.addr vs server.port collision check used eq on incompatible types. First version compared a string (:8081) to an int (8080) — eq always false, guard never fired. Fixed by stringifying server.port to printf ":%d" before comparison.
• Shell-script smoke embedded an apostrophe inside a single-quoted sh -c '...'. "the resource server's identity" / "the broker's OIDC handler" — bash parser tracked it as unmatched-quote EOF. Reworded both comments to drop apostrophes. Lesson: when writing shell-inside-shell, write smoke text without contractions or use heredoc-quoted (<<'EOF') rather than sh -c '...'.
• Pre-existing deployment_test.yaml assertion pinned :0.1.0 image tag. Chart.yaml bump to 0.2.0 cascaded — updated the test to match. Existing chart pins like this are an argument for using .Chart.AppVersion in tests via match-anything-version regex instead of pinning a literal, but that's a sweep for later.

Scope outcome vs plan estimate

Over the original ~430-LOC budget but well-explained: the cross-template helper + ingress-topology shape + parallel cert-manager Certificate + 3-check smoke each pulled in real lines. The 24 new unit-test cases pin one invariant each (default render, override flow-through, TLS volume mount, MCP port admission, two-host ingress, parallel Certificate, fail-render guards).

Test/lint posture: helm unittest deploy/helm/demarkus-broker → 9 suites, 95 tests, all green. bash pre-commit.sh → fmt + vet + golangci-lint × 4 modules, all green.

Status after this slice

• 7 of 8 slices shipped. Plan's only remaining slice is Slice 8 (/knowledge-join plugin slash command + plugin docs). Slice 8 is the user-facing onboarding closer and is also the natural place for the full id_token / initialize / tools/call E2E that Slice 7's smoke deliberately deferred.
• The kind harness now has a --with-mcp-smoke flag that proves the chart's MCP listener wiring against a locally-built broker image. Three checks (metadata × 2, auth challenge × 1) — fast feedback loop for future chart changes.
• Chart README + MCP-API.md give operators and plugin developers a single place to read the gateway's contract. The ephemeral graph-store gap is documented prominently (operators should expect re-crawl after restart).

Next session — Slice 8 starting point

• feat-plugin-claude-code-knowledge-join branch (or similar).
• plugins/claude-code/commands/knowledge-join.md — prompt-shaped slash command that takes a broker URL.
• Validate via HEAD <url>/.well-known/oauth-protected-resource (the metadata endpoint Slice 7 just stood up).
• Derive slug from hostname, run claude mcp add --transport http {slug} {url}/mcp.
• Bump plugins/claude-code/scripts/lib.sh SERVER/CLIENT/TOOLS_VERSION pins per feedback_plugin_version_pins.md (Slice 7 was chart-only so no plugin bump was warranted; Slice 8 ships plugin-visible behavior).
• The full E2E test (id_token → initialize → tools/call) belongs in Slice 8 — the slash command exercises the path naturally.