Build the MCP tool surface from transport providers - #80
Merged
Conversation
This repo's own project config records the maintainer's default keg and flight. Those are per-developer choices, not shared state, so the directory should never have been commitable in the first place. Also ignores *.iml alongside the existing .idea/ entry.
ConfigService kept four independently-invalidated caches (user, project, merged, warnings) behind a `cache bool` threaded through every read. The flag made each call site restate a question it had no basis to answer, and the caches could disagree with one another mid-process. Configuration is now read once and fixed for the life of the process. A `tap` command runs against one consistent snapshot; a long-lived `tap mcp` session picks up an external edit at its next orient, which is the only place Reload is called. Nothing inside a session can write configuration, so "edit the file, then reorient" is the whole update story. The snapshot is immutable once published, so concurrent readers need no coordination beyond the mutex guarding the pointer. That matters because the MCP SDK dispatches every call except initialize asynchronously. Read-modify-write paths that rewrite a config file need the bytes on disk rather than the snapshot, so they call ReadUserConfigFile / ReadProjectConfigFile and then Reload once the write lands. Load() returns warnings alongside the merged config instead of leaving them on a struct field that only happened to be populated by the last Config() call.
The root command copied the configured flight into KegTargetOptions on every invocation, which meant a persisted `flight:` silently narrowed keg access for direct commands like `tap cat` and `tap edit`. That contradicted the documented rule that direct CLI commands are governed by normal keg authorization rather than flight cover, and it made the configured selection indistinguishable from an explicit --flight once it reached resolution. Selection now happens where it is meaningful: ActiveFlightName reads the snapshot when orientation asks for it, and --flight remains the only way to put a flight into per-command keg targeting. A flight persisted in user or project config therefore no longer narrows direct CLI commands. Pass --flight explicitly to opt a single invocation into flight cover enforcement.
ImportFromKeg resolved the source keg at viewer regardless of options, but leave_stubs rewrites source nodes to point at their new home. A viewer-only flight could therefore mutate a keg it was only authorized to read. The source role now tracks what the operation actually does: viewer for a plain copy, editor when leave_stubs is requested.
NewServer branched on a Surface enum to decide which tools to register, which meant every capability difference between `tap mcp` and the hub's /mcp endpoint was encoded as "which enum am I". The enum conflated things that vary independently — who the caller is, which catalog backs discovery, and whether the server shares a filesystem with its agent — so adding a transport meant auditing every branch. Transport differences are now expressed as four interfaces: OrientationProvider selects and renders flight authority, FlightProvider supplies flight CRUD, KegDiscoveryProvider lists reachable kegs, and IdentityProvider reports who the session is. Nil providers fall back to local adapters over tap; the hub injects authenticated, catalog-backed implementations. Registration itself no longer branches. Two consequences for the published surface: - auth_status gives way to auth_info, which returns structured identities and flight-filtered kegs and is deliberately credential-free. Tokens, email, scopes, cookies, expiry, and session data are absent from the wire shape by construction. - keg_list drops its hub selector and filters through the session's immutable active-flight cover, so discovery cannot report kegs the flight excludes. A session may now edit or delete its own active flight. The previous blanket prohibition meant a flight granting manage_flights could administer every flight except the one it most likely needed to correct. A successful self-edit adopts the exact returned manifest before the response is released, so removing manage_flights removes the mutation tools immediately; a self-delete enters recovery immediately. If the edit persists but rendering fails, the session enters recovery rather than retaining stale authority. Attachment transfers split into two variants behind ServerOptions.SharedFilesystem, since only stdio shares a filesystem with its agent host. The hosted variant omits local-path fields from the published schema rather than refusing them at call time, so a hosted agent has no vocabulary to name the server's own disk. `tap mcp` opts in separately. config, config_template, license, repo_init, export, import, namespace_list, and keg_visibility leave MCP entirely: they operate on machine-local Tapper state or perform tenant administration. The parity coverage map records the rationale per method. Migration notes for embedders: mcp.Surface, SurfaceFull, SurfaceHub, ServerOptions.Surface, ServerOptions.OrientationLoader, and ServerOptions.LicenseText are gone; construct provider implementations instead. The nine tools listed above are no longer registered on any transport.
`tap mcp` runs on the same machine as the agent driving it, so a path in a tool argument names the same file for both sides. It now opts into the shared-filesystem variant and regains the full attachment round-trip: upload_file and upload_image accept source_path and file: URIs alongside byte sources, download_file writes to dest_path, and download_image takes an optional dest_path, still returning MCP image content when omitted. The option set is a named function rather than an inline literal so the choice is assertable without standing up a stdio server. Dropping it would silently cost `tap mcp` these transfers while leaving every other test green, which is exactly how they were lost.
jlrickert
force-pushed
the
refactor/mcp-transport-providers
branch
from
August 4, 2026 05:11
e5df295 to
91b2130
Compare
Exercising the MCP and flight changes by hand means repeatedly starting
Claude Code or Codex against a chosen model with a chosen flight, and
making the flight stick means exporting TAP_FLIGHT or editing config
between runs.
`tap launch HARNESS --agent NAME` collapses that. An agent aliases a
model, a flight, an endpoint, and how to authenticate:
agents:
local:
model: ollama/qwen3.6:35b-mlx
baseUrl: http://192.168.50.197:11434/v1
flight: '@homelab/+ecw'
sub:
model: anthropic/claude-opus-4
auth: subscription
work:
model: openai/gpt-5
apiKeyEnv: WORK_OPENAI_KEY
Models are provider-qualified because the provider decides which protocol
the harness must speak; an unqualified model is rejected rather than
guessed at. Ollama serves both the OpenAI API and the Anthropic Messages
API, so it is the one provider every harness can drive — Claude Code
included. Codex against a hosted Anthropic model, and Claude Code against
a hosted OpenAI model, stay refused before anything is spawned.
One baseUrl serves both protocols: the launcher appends /v1 for OpenAI
clients, which add /chat/completions, and strips it for Anthropic ones,
which add /v1/messages themselves.
Auth is explicit because absence cannot express intent — an unset key
means "inherit", which silently prefers an exported API key over a
subscription login. `auth: subscription` therefore removes the inherited
provider key variables from the child environment, which an overlay
cannot do: appending can override a variable but never unset one.
apiKeyEnv names the variable holding a key, never the key itself,
mirroring HubEntry.TokenEnv, so agents hold no secrets and need no
trust-boundary strip in project config.
The agent's flight is exported as TAP_FLIGHT, which needs no new plumbing
— it already outranks project and user config in the flight chain, so a
tap mcp session started inside the harness orients to it.
Launch is ResolveLaunch plus execution, so --dry-run and a real run
cannot drift, and the whole surface is testable without spawning a
harness.
Registered under the existing IncludeIntegrations profile gate, so the
pruned keg binary does not grow the command. Excluded from MCP surface
coverage: spawning processes on the server's host is not an agent
operation.
Experimental and intentionally undocumented — this integrates with
Tapper Hub later and will be redesigned, so docs and the config template
are deliberately untouched.
jlrickert
force-pushed
the
refactor/mcp-transport-providers
branch
from
August 4, 2026 05:57
74a8153 to
7881327
Compare
A session that cannot establish flight authority had almost no way to say so. The tool list is filtered to the recovery set, so an agent never gets to call a locked tool and see the error explaining why — leaving an empty KEG table as the only signal. Models strong enough to reason from an absence recovered; weaker ones did nothing at all. The two recovery paths also had their messages crossed. A blank selection produced the generic orientation document, which never mentions flights being locked. A selection that failed to resolve produced "no flight is selected", which is false and sends the reader hunting for missing configuration instead of the real fault — usually a wrong flight name or an unreachable hub. Now the payload carries a Flight section whenever no flight is active, naming the state and the three steps out of it, and a failed resolution leads with the actual error. Both stay within the payload's MCP-first rule of never naming CLI commands. The orient tool description carries the same imperative, because tool descriptions are the one thing every model reads; server instructions alone are not enough for a small local model.
graph rendered a standalone HTML page — DOCTYPE, stylesheet, and an embedded JavaScript renderer — and returned the whole document as tool text. An agent cannot display it, and the part it could actually use, the node and edge JSON, was buried inside a script tag. The static scaffolding alone ran to roughly 8KB before any nodes, so every call spent context on markup nobody reads. The MCP and CLI renderings had also drifted: GraphOptions.BundleJS is set only by the CLI, so MCP fell through to the inline fallback bundle and produced a visibly different page from the same data. Unregistered rather than deleted outright — Tap.Graph and `tap graph --output` still serve the case that works, writing a file to open in a browser, until the feature is removed. The tool row is dropped from the embedded tool inventory too: that text ships inside the orientation payload, so leaving it would advertise a tool that no longer exists.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the MCP
Surfaceenum with transport-shaped interfaces, and fixes theconfiguration cache it was tangled with.
Why
NewServerbranched on aSurfaceenum to decide which tools to register, soevery difference between
tap mcpand the hub's/mcpendpoint was encoded as"which enum am I". That conflated things that vary independently — who the
caller is, which catalog backs discovery, and whether the server shares a
filesystem with its agent — so adding a transport meant auditing every branch.
Separately,
ConfigServicekept four independently-invalidated caches behind acache boolthreaded through every read. The flag made each call site restate aquestion it had no basis to answer, and the caches could disagree mid-process.
What changed
Configuration is read once and fixed for the life of the process. A
tapcommand runs against one consistent snapshot; a long-lived
tap mcpsessionpicks up an external edit at its next orient, the only place
Reloadis called.The snapshot is immutable once published, which matters because the MCP SDK
dispatches every call except
initializeasynchronously.MCP registration no longer branches. Four interfaces carry the transport
differences:
OrientationProvider,FlightProvider,KegDiscoveryProvider,IdentityProvider. Nil providers fall back to local adapters overtap; the hubinjects authenticated, catalog-backed implementations.
Surface changes:
auth_statusbecomesauth_info, returning structured identities andflight-filtered kegs. Credential-free by construction: no tokens, email,
scopes, cookies, expiry, or session data in the wire shape.
keg_listdrops its hub selector and filters through the session's immutableactive-flight cover.
prohibition meant a flight granting
manage_flightscould administer everyflight except the one it most likely needed to correct. Self-edit adopts the
exact returned manifest before the response is released; self-delete enters
recovery immediately.
config,config_template,license,repo_init,export,import,namespace_list, andkeg_visibilityleave MCP. They operate onmachine-local Tapper state or perform tenant administration. The parity
coverage map records the rationale per method.
Attachment transfers split into two variants behind
ServerOptions.SharedFilesystem, since only stdio shares a filesystem with itsagent host.
tap mcpkeeps the full round-trip —source_path,file:URIs,download_filetodest_path, anddownload_imageto either a path or inlineMCP content. The hosted variant omits those fields from the published schema
rather than refusing them at call time, so a hosted agent has no vocabulary to
name the server's own disk.
Migration notes
For embedders of
pkg/mcp:mcp.Surface,SurfaceFull,SurfaceHub,ServerOptions.Surface,ServerOptions.OrientationLoader, andServerOptions.LicenseTextare gone — construct provider implementationsinstead. The nine MCP tools listed above are no longer registered on any
transport.
For CLI users: a flight persisted in user or project config no longer narrows
direct commands. Pass
--flightexplicitly to opt one invocation into coverenforcement. This restores the documented rule that direct commands are governed
by normal keg authorization.
Also included
import_from_kegresolved its source at viewer regardless of options, butleave_stubsrewrites source nodes — a viewer-only flight could mutate a keg itwas only authorized to read. The source role now tracks what the operation does.
Verification
go build ./...andgo test ./...are clean, and each of the six commitsbuilds and tests green in isolation.
Test coverage worth calling out: the two attachment surfaces are pinned from
both sides —
TestMCP_LocalSurfacePublishesLocalPathTransfersassertstap mcppublishes the local-path vocabulary, and
TestMCP_UploadSchemaRejectsLocalSourcePath/TestMCP_DownloadImageSchemaRejectsDestPathassert the hosted schema omits it.TestMcpServerOptionsSharesFilesystemcovers thetap mcpopt-in itself, whichwould otherwise be deletable with the whole suite still green.
Also included:
tap launch(experimental)tap launch HARNESS --agent NAMEstarts Claude Code, Codex, or pi with themodel and flight named by a configured agent. An agent is an alias for a
(model, flight) pair; the flight is exported as
TAP_FLIGHT, which alreadyoutranks project and user config, so a
tap mcpsession started inside theharness orients to it. This exists to make the rest of the PR easier to
exercise by hand.
Models are provider-qualified so the launcher knows which protocol the harness
must speak, and incompatible pairs fail before anything spawns rather than
launching something broken.
LaunchisResolveLaunchplus execution, so--dry-runand a real run cannot drift.Deliberately undocumented and marked experimental in code: it integrates with
Tapper Hub later and will be redesigned, so
docs/**and the config templateare untouched.
Follow-ups not in this PR
license/config/config_template/repo_init/ archive tools. They should become deletionsonce the surface reduction is settled, and
tools_archive.go,tools_repo.go,tools_namespace.go, andtools_license.goare now dead.tapper-hubhas the matchinginternal/handler/mcp.goproviderimplementation, which needs a release to pin against before it can land.