Skip to content

feat(mcp): hosted formae support (slice 2b) - #34

Open
JeroenSoeters wants to merge 16 commits into
mcp-one-installfrom
mcp-hosted-credentials
Open

feat(mcp): hosted formae support (slice 2b)#34
JeroenSoeters wants to merge 16 commits into
mcp-one-installfrom
mcp-hosted-credentials

Conversation

@JeroenSoeters

Copy link
Copy Markdown
Contributor

Turns hosted on. formae connection resolve becomes the MCP's single
configuration read, the resolved credential reaches exactly one request header,
and the failure semantics around it are the ones the design specifies rather
than whatever HTTP happens to do.

Stacked on #33 (mcp-one-install), which stacks on #32. Draft until 0.89.0 is
released and the stack is tested end to end against it.

Plan: "Plan: Hosted support slice 2b (the MCP half)" on PLA-428. Producer side
is formae #654.

What is here

  • Installations are KSUIDs. The control plane mints them that way and the
    edge routes on ^[0-9A-Za-z]{27}$; this validator still required a canonical
    lowercase UUID, so it rejected every real installation. The same fix landed
    on formae #654, where three validators had the old grammar. It stays a shape
    check rather than a decode, so this client cannot be stricter than the router.
  • secret.Value, masked in every rendering path. Writing its test found
    that fmt cannot reach a Format method on an unexported field, so a
    struct holding one prints the credential under %v regardless. The routing
    masks itself and the limit is documented where the type is defined.
  • One read for configuration and credential. New argv (--profile is a
    local flag and follows the subcommand, unlike profile show), a flat
    document, and failure envelopes decoded off the same stdout. The producer's
    free-text message is never surfaced: it is built from an auth plugin's error
    or an arbitrary err.Error(), and a Pkl failure quotes profile source lines,
    which for a classic profile can mean an inline password. details.pluginCode
    is validated rather than trusted, because the producer copies it through
    unchecked from a bare string alias.
  • Hosted routing as a sum with the base URL inside each arm, so no value
    can pair one arm's endpoint with the other's authentication. Every redirect
    refused: Go strips Authorization across a cross-host redirect but forwards
    custom headers.
  • Retryability is a required argument, not a defaulting field, so a new
    call site cannot inherit the wrong answer. A 401 refreshes and verifies mode,
    origin and installation against the snapshot before retrying once; mutations
    refresh and never retry; 403 has no branch at all.
  • A hosted 404 on a collection is a routing error rather than an empty list.
  • Ambiguity becomes an instruction naming the candidates.
  • Results say where they went, in a separate block, worded by how far the
    call actually got.

Review

Four adversarial passes on the plan before any code, then four on the diff.
Every finding was accepted; the notable ones:

  • Hosted response bodies were returned verbatim, so an intermediary echoing
    request headers would have put the bearer token into a tool result. The
    routing now scrubs every credential it sent, including one a retry replaced.
  • The attribution claimed the installation answered whenever any response
    arrived. Every response comes from the shared edge, and a hosted 404 says two
    lines away that the edge did not route it, so the two contradicted each other
    in the same result.
  • reachAttempted was set before Do, so a connection refused told an operator
    to check for work that was never sent. It now comes from the request actually
    being written, and a partial write counts.
  • Extract's failure output bypassed the scrub entirely and was unbounded.
  • safeSubprocessOutput truncated before scrubbing, leaving a credential that
    straddled the cutoff mostly intact.

Behaviour claims are mutation-tested; each mutant was confirmed to compile
before its result was believed.

Not here, deliberately

Elicitation and a remembered per-session profile. The instruction works in
every MCP client, needs no session state, and is what elicitation falls back to
anyway.

Gates still open

  • Integration evidence against the registered installation. The fleet agent
    has provisioned 3HzFPXfPDGhwLJJVtaHbmFs6vLa, but the edge does not route it
    yet: /api/v1/health returns 404 with that header, with a UUID header, and
    with no header alike. Confirm it is in the routing feed before reading a 404
    as an MCP defect.
  • Resolution latency, measured per call, under concurrency and across a
    refresh. Hosted resolution spawns formae, which spawns the auth plugin, on
    every tool call.
  • The installation authorization boundary, which the design's account of
    credential skew infers from the claims schema rather than observation.

The control plane mints installation ids as KSUIDs and the edge routes on
them, so the canonical-lowercase-UUID grammar rejected every real
installation. The check stays a shape check rather than a decode, so this
client cannot end up stricter than the router it talks to; a test pins that
limit deliberately.
The invariant that no path writes a credential into a result, an error, or a
log cannot be established by a test, because the next handler someone adds is
not covered by it. It becomes a property of the type instead: String, Format,
MarshalJSON, MarshalYAML and LogValue all mask, the field is unexported so a
reflection-based encoder sees nothing, and Reveal is the single greppable way
out.
The oracle moves from `formae profile show` to `formae connection resolve`,
which produces both from one evaluation of one profile. Two independently
timed reads could not: between them the active pointer can move or the profile
can be rewritten, and a request would carry one revision's endpoint with
another's credential.

Three things about the new contract are easy to get wrong and are pinned by
tests. --profile is a local flag and follows the subcommand, where profile
show took the name positionally. The document is flat rather than nested under
cli. A declared failure is an envelope on the same stdout, so a non-zero exit
is read rather than reported blind.

The producer's free-text message is never surfaced: it is built from an auth
plugin's error string or an arbitrary err.Error(), and a Pkl failure quotes
profile source lines, which for a classic profile can mean an inline password.
Only the code and a validated plugin code cross over.
The client's routing becomes a sum with the base URL inside each arm, so no
value can pair one arm's endpoint with the other arm's authentication and a
classic client has no field a credential could occupy. Hosted sets
Formae-Installation exactly once and carries the credential; classic carries
neither and behaves as before.

Every redirect is refused on hosted, not only a cross-origin one: Go strips
Authorization across a cross-host redirect but forwards custom headers, so the
routing header would otherwise follow one. The policy hands the 3xx back
rather than erroring, which keeps the status available and leaves nothing
wrapped for a caller to unwrap wrongly.

Writing the containment test found a hole worth recording: fmt cannot reach a
Format method on an unexported field, so a struct holding a masked value
prints the credential under %v regardless. The routing masks itself, and the
limit is now documented and pinned where the type is defined.
Retryability is a required argument rather than a field with a default, so a
new call site cannot inherit an answer by leaving a zero value alone. Every
bodyless GET is retryable; every mutation refreshes so the next call succeeds
and then returns the original failure.

A 401 re-resolves with the credential refreshed and then compares mode,
origin and installation against the snapshot the call started from. Without
that comparison the refresh path silently re-introduces the skew that
resolving configuration and credentials together exists to prevent, and a
retry could read from a different installation than the one addressed. 403 has
no branch at all: a denied installation is not an expired session.

A retryable request may not carry a body. The first attempt consumes the
reader, so a retry would send an empty one, which looks like a successful
request rather than a corrupted one.
The shared edge answers 404 for an unknown or unrouted installation, so the
six endpoints that translate 404 into an empty payload would present a routing
failure as a plausible answer, which is the failure mode nobody investigates.
Under hosted they become an error naming the installation they addressed.

GetCommandStatus keeps its 404: that one is an object not-found, and telling
edge-routing 404s from agent-object 404s properly needs a stable edge error
envelope that does not exist yet, so the narrowing stops where the ambiguity
starts.
…ruction

The CLI decides ambiguity, since it is the only side that can settle it before
a credential is minted. This pins that the refusal arrives as a message naming
every candidate and the active one, and asking for the profile argument, so a
caller can act on it rather than seeing an opaque failure.

Elicitation and a remembered per-session profile are deliberately not built:
this works in every MCP client, needs no session state and no capability
negotiation, and is what an elicitation path falls back to anyway.
A hosted result names the effective profile and the installation in a second
content block, never by touching the payload a consumer parses. Classic
carries none: the user already knows which agent they pointed at.

The wording follows how far the call actually got, which only the executor
knows. An agent 500 answered. A mutation whose transport failed after dispatch
was addressed and may already have acted, and saying it did nothing would be
confidently wrong. A forma file that failed to evaluate resolved a destination
and sent nothing, and claiming otherwise would send an operator to check an
installation for work that cannot exist.

The policy planners are agent-backed despite the name, and are the one place
where 'answered' cannot be read off the result, because an unreachable agent
is deliberately swallowed there. fetchPolicies reports its own reach instead.
That also makes a pre-existing gap visible under hosted: those tools take no
profile argument, so their inventory read is ambiguous with several profiles
and the ambiguity is swallowed like any other failure.

The structural check enforces this per return rather than per handler. The
first version asked only whether a handler mentioned attribute() anywhere, and
a mutation proved it passed on a handler whose success path had none.
…ver-claiming

Three findings from the branch review, all verified in the code.

A hosted response body was returned verbatim and interpolated into errors, so
an intermediary echoing request headers would have put the bearer token into a
tool result and from there into a model's context. The routing knows what it
sent, including a credential a retry replaced, and scrubs it on the way back.

The attribution claimed the installation answered whenever any response
arrived. Every response comes from the shared edge, and a hosted 404 on a
collection says two lines away that the edge did not route it, so the two
contradicted each other in the same result. It now says the endpoint answered.
Attempted comes from the request actually being written rather than from being
about to call Do, so a connection refused no longer tells an operator to go
and check for work that was never sent.

Responses are bounded like the configuration oracle's output already is. An
unbounded read from a peer is an unbounded allocation, and that peer is now
remote and shared rather than a process on this machine.
… sent

Two findings from the second review pass.

Extract reaches the agent through the CLI, so its output never passed through
the executor's scrub and went into a tool result whole. Its diagnostics are
the value of the failure, so they are kept, bounded, with the credential we
handed the process removed. A credential the CLI minted for itself is not ours
to know and is the CLI's own to mask, which the comment says rather than
implying the output is sanitised.

WroteRequest can report an error having already put part of a multipart
mutation on the wire. Counting only the clean case told an operator nothing
was sent when something may have been. Any invocation of the callback now
counts; DNS, connection and TLS failures never reach it, which is the
distinction it exists for.
Truncating first leaves a credential that straddles the cutoff partly intact:
the search string is no longer present in the truncated bytes, so nothing is
replaced and all but the tail of the token survives. The test walks the
credential across the boundary a byte at a time.
An installation can disappear underneath a live session: a trial ends, a
subscription lapses, someone suspends or destroys it. Sessions stay open for
days, so this is an ordinary event, and when it happens every call fails at
once.

Only the six collection endpoints explained it. The rest asserted their own
reading of a 404 — an unhealthy agent, a bare status with the edge's body
pasted in, and worst, a command that was not found, which sends the reader
hunting for something that was never the problem.

The endpoints whose 404 has no object reading now report the routing failure
outright, since the agent answers those with 200 or an error and never 404.
get_command_status reports both readings, because the status alone cannot
separate them and asserting either would be a claim this cannot support.
Classic is untouched: a self-hosted agent answers for itself, so its 404 means
what the endpoint says it means.
@JeroenSoeters
JeroenSoeters marked this pull request as ready for review August 28, 2026 22:15
Its comment says it exists for tests, but no test calls it: the hosted
test clients are built by newTestHostedClientAt. Dead code that fails
the unused linter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant