feat(mcp): reconcile sources and refresh session tools

[JAORMX]

Stage

Implementation — stacked on #1527

Contract and baseline

• Plan / Interface PR: docs: plan MCP source reconciliation #1527.
• Exact amended plan baseline: 6ad42a98127e6a5bf18246dde8f83db909fb4b1a.
• Rebased onto main through ca993a6ea1b648e8e18a102b6810da5dea8c494f.
• The directing human explicitly authorizes implementation before plan merge and approved the narrow cleanup API/storage rollback amendment. No merged approval is claimed.
• This PR targets plan/mcp-source-reconciliation. Humans merge the plan first, then retarget/review this implementation against main.
• Interfaces match the explicitly authorized amended contract, including additive Supervisor.Close() / ErrSupervisorClosed. Proposed ADR: 0351.

Summary

• Reconcile ToolHive workloads and MCP tool/resource/prompt notifications through a bounded, coalesced source reconciler.
• Publish immutable direct-MCP runtimes; pin active runs and resource/prompt operations until settlement, with bounded retirement and revision-based engine rebuilding.
• Preserve exact-name authority: removed names become unavailable, previously granted names can reappear, and new names require explicit owner refresh.
• Add Session.GrantToolAuthority, direct gRPC/HTTP refresh controls, cached revision/status, SDK support, and unified mecatui /mcp-refresh routing.
• Keep broker consent/cancellation separate and /tools-connect as a deprecated broker-only alias.
• No durable capability digests, revoked records, snapshot migration, or new endpoint-credential policy.

Review repairs

• Fixed unpinned runtime/OAuth reuse and generation identity, including construction-time publication races, pinned revision zero, and published empty runtimes.
• Unified direct-team declaration and run-time construction; atomically publish startup state; prevent stale registry claims and cancellation panics.
• Added exact-once supervisor teardown, including concurrent close and close from event callbacks. Failed startup rolls back member snapshots under mutation authority with bounded cancellation-detached cleanup; unsupported rollback storage is rejected before enrollment.
• Moved refresh reconciliation after ownership/lifecycle validation and the per-session lock; sanitized load/reconcile/save errors through Service and HTTP/gRPC.
• Preserved notifications across candidate handoff, failed builds and deferred publication; bounded reconnect cycles with a one-second cooldown.
• Validate remote tool names without rewriting valid identities.
• Updated lifecycle, rollback/recovery and refresh API documentation and regression coverage.

Acceptance and verification

All 13 amended ACs have proof mappings. The plan's landed status in this diff is a proposed transition, effective only when this implementation merges.

Passed during repair verification:

• Full task lint and task test at earlier repair checkpoints.
• Final targeted race regressions, including repeated lease-loss/refresh-lock tests.
• task api:check, task docs, task site:build, and offline go run ./cmd/mecademo at repair checkpoints.
• MCP acceptance trace: 13 ACs, zero failures.
• git diff --check.

Fresh CI on 4b2f27e1d4b05150e62d5456f044a39e8b6bad96 is green: CI, Kubernetes e2e, performance, and advisory checks passed. This includes lint/analysis, race suites, macOS temporary-storage/CLI checks, SDK matrix, standalone modules, docs, API compatibility, build, and vulnerability checks. Conditional live-model tests were skipped. The first push's receiver-lint and macOS test-ordering failures were repaired and are superseded by these results.

Independent reviews covered spec, standards, test adequacy, security, architecture, UX, DevEx and reuse. Confirmed lifecycle/runtime findings were repaired with regressions.

Explicit residuals

• Remote description/schema prompt hardening remains nonblocking, as in the original review. Aggregate ingestion bounds are not semantic prompt-injection protection. This change preserves specifications; blanket schema rewriting/truncation would corrupt valid tool contracts. A separate metadata-provenance/projection policy is needed.
• Snapshot rollback is best-effort after lease loss or deletion failure: retain state rather than deleting without authority. The documented retention/recovery path applies.
• The refresh-lock concurrency test retains a bounded negative observation; it is not claimed to be a fully deterministic scheduler oracle.

Tracking: #1511

[jhrozek]

The reconciler core here is genuinely solid work - immutable candidates, atomic swap, LKG, all-or-nothing publication, and a test suite that exercises the real mcp.Manager and the real session factory rather than stubs. Verified: the shared-manager lifecycle bullet holds (nothing folds the runtime Close into per-session close), and TestPerSessionCatalogMatchesSharedCatalog + Scenario3_NameAuthorityAvailabilityMatrix both prove the catalog-equality and mid-session-tool-change guarantees you asked about.

Four things need fixing before merge (flagged inline below, all independently traced against this commit, not just taken on the reports' word):

A. An unpinned ctx on the operation-revision path breaks the shared-engine fast path after the first MCP republication, and reaches the MCP-authorization continuation specifically.
B. CreateTeam/RunTeam now have two lifecycle implementations gated on OperationPin, and production only ever exercises the less-tested one.
C. A nil-pointer panic is reachable in CancelTeammate - ts.phase flips to teamRunning before ts.sup is actually built on the pinned path, and CancelTeammate doesn't take any lock that would serialize against that window.
D. RefreshMcpSources triggers shared reconciliation one line before it acquires runEntryMu, ahead of the acceptance plan's own documented linearization point.

A few more items worth a look before/after this lands, not blocking:

• internal/adapter/mcp/tool.go's newRemoteTool carries tool descriptions/schemas into the model's system prompt verbatim, unfenced - this file isn't touched by this diff, but automatic ToolHive publication (this PR's whole point) newly makes that a live prompt-injection surface where it wasn't before (no more operator restart/consent gate). Worth a governance.NeutraliseFraming pass plus a length clamp in a follow-up.
• mcp_refresh.go's error path interpolates raw backing-store failure text into ErrInternal, serialized over both gRPC and HTTP - could leak backend paths/addresses.
• Malformed remote MCP names (control chars, invalid UTF-8) can reach durable root authority without the validation explicit refresh grants enforce (toolhive.go's validation only checks __; remote tool creation only checks non-empty).
• Real data race: teamState.phase is read under one lock in SendTeammateMessage and written under a different lock discipline in RunTeam's deferred cleanup.
• list_changed notifications can be silently and permanently lost in two separate windows (the preparingGen to current gap, and a deferred-publication path that discards candidateDirty) - worth fixing since notification delivery is this feature's actual purpose.
• A remote MCP server flooding list_changed can drive an unbounded reconnect storm (no backoff between reconcile cycles).

Happy to open follow-up issues for the non-blocking items if useful.

[JAORMX]

Pushed the rebased stack and review repairs. Current implementation head: 1f91831a1830aafc19bab24be4f6f8dd239cea5b; amended plan baseline: 08954175e8393f506d23c7575e59e29a551aefb2 (#1527, proposed ADR 0350).

All four inline findings have been addressed:

1. Operation revision / OAuth continuation: pinned-first resolution falls back to the live runtime only when no pin exists; shared manager/revision are captured together during construction. Regression coverage includes OAuth engine reuse, pinned zero and published empty generations.
2. Team lifecycle: Create/Spawn are declarations in every configuration, with member construction at Run entry. This timing is documented and independently covered. Cleanup checks current mutation ownership before deleting snapshots. The directing human approved additive Supervisor.Close() and the rollback-storage prerequisite; the plan/API snapshot/changelog are amended together.
3. CancelTeammate panic: starting is distinct from running; team and supervisor publish atomically. Startup also revalidates registry identity so successful cleanup cannot be followed by an orphaned run.
4. Refresh linearization: owner/lifecycle checks and the per-session entry lock precede reconciliation. No-op refresh stays write/lease-free, and additive grants retain save confirmation/ambiguity handling.

Related feedback: fixed phase locking and private-error leakage; validated remote names; retained dirty notifications through handoff/failure/deferred publication; added a one-second reconnect cooldown. Additional review caught and fixed partial-start resource leaks, concurrent/callback teardown races, and build-time runtime-tag mismatches. Regression coverage includes actual HTTP/gRPC error projection and automatic runtime-drain publication.

Explicitly not claimed fixed: stronger remote description/schema prompt trust. Existing byte/page/entry bounds do not neutralize instructions. Blanket schema sanitization would change valid tool contracts; this remains the review's nonblocking follow-up requiring an agreed metadata-provenance/projection policy. Snapshot rollback also remains fail-safe/best-effort when lease ownership is lost or deletion fails, with retention guidance documented.

Targeted race tests are green. Earlier full repair gates passed; latest full local reruns encountered the embedded-scheduler TUI race and host contention, so I am relying on fresh pushed-head CI for final verification and monitoring it to completion.

[jhrozek]

Review follow-up: what changed and why

I've taken over this PR. These 8 commits fix the review findings I posted earlier (review 5290674823). Each inline thread has a reply with the details and is resolved.

Fixes

• Refresh no longer sends upstream error text to clients (a126fd8). Failed candidate builds could put server names, host:port and upstream response text into the gRPC status message and the HTTP error body. AC1.3 and grpc-api.md both promise generic errors. RefreshMcpSources now logs every non-context reconcile error for the operator and returns a generic ErrInternal. The fix is in the Service, not the Build closure, so any MCPRefresh implementation gets it.
• DebugMCP follows the current runtime (0c738da). It was computed once from the startup manager, so it went stale after the first republication. It's now a function read on every capabilities request.
• Docs (752ee9c, 6e2f70b):
  • AGENTS.md describes pinned runtime ownership instead of one Build-owned manager.
  • ADR 0027 cites ADR 0351 (it said 0350).
  • PRODUCTION-READINESS.md has a row for this feature.

Tests that couldn't fail before

Each one below now fails when the production behaviour it covers is removed. I checked each by breaking that line.

• Reconciler shutdown (78e0efb): the test now fails if Close stops waiting for in-flight reconciliation.
• Resume after approval (5dcb722): new test for approving a parked call after its granted mcp__ tool has left the catalog. It expects the exact "currently unavailable" result.
• Team fork cleanup (6b7ee67): the fake no longer hides a double cleanup behind sync.OnceValue.

Decision: shared engine

After the first republication, default-FS sessions move to their own per-session engines, and the shared engine is never rebuilt. The only run path to the shared engine checks revisions, so this is a cost issue, not a correctness bug. For this PR, ADR 0351 decision 6 now documents it as the one exception to "rebuilt before use" (854eee3). Rebuilding the shared engine once per revision is tracked in #1802.

Verification

• Lint: golangci-lint reports 0 issues on the root and engine modules.
• Tests:
  • Engine module go test -race ./... passes.
  • The GOWORK=off standalone engine check passes.
  • Root module: task test passes except cmd/mecatui/agenthook TestDefaultRunnerDeliversPayloadAndHarness, which timed out under machine load. That package isn't touched here, and the test passes 3/3 run alone.
• Other gates:
  • task api:check passes.
  • The docs link gate passes on a clean export of the branch.
  • mecademo prints a full offline session.

Not in this PR

These came up in review and are left out on purpose:

• the team lifecycle rework in internal/adapter/server/team.go
• removing globalMgr from catalogAssets and merging the duplicate …ForOperation config seams
• approval pins filling the retirement slots
• Unicode format characters in tool names
• the triplicated bounded-list loops

🤖 Generated with Claude Code