Two defects in internal/mcp, both about a call that reports done when
nothing happened.
Client.call accepted a frame carrying our id and neither result nor
error. The HTTP transport already refuses one; the stdio transport does
not, so the refusal depended on which door the server was behind. Down
that path tools/call returns an empty string and a nil error, and the
act is recorded as run.
Manager.ReadResource reported ErrNoServer for a server that is
configured but down. Call keeps those two apart on purpose — one says
the tool can never exist, the other says not right now.
Call, ReadResource and CallPositional each opened the connection map by
hand, and Call took the mutex twice to answer one question. They now
share lookup, which returns the client, the config and the tool in one
critical section.
dial did three things. Choosing the transport is openTransport, and
recording a live connection is succeed, so the function reads as
handshake then discovery.
Also: argv is a method rather than an append repeated in dial and
Status, the transport strings are constants, and the tail binding comes
out of bindPositional as bindOne. The comment on filterTools claimed it
drops nameless tools, which it never did.
No behaviour change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Refresh called alive() with the manager lock held, so a slow health
check blocked every other server. It now snapshots the candidates and
asks outside the lock.
A server that cannot be dialled was retried every minute forever, which
for a misconfigured stdio block means re-exec'ing a process 1440 times a
day. Dials now back off from one minute to thirty.
An allow_private fetcher followed redirects. A LAN MCP endpoint could
answer a POST with a redirect to 169.254.169.254 and the guard would go
there, because allow_private is what turns the address check off.
Redirects are refused outright on that door.
The tool catalogue was trimmed by taking the first max_tools entries of
whatever order the server sent, so the server chose which of its tools
Maven proposed. Over the cap without allow_tools now contributes
nothing: refusing is honest, silently keeping the server's pick is not.
Descriptions are server-written text that lands in the router prompt and
on /tools, so they are capped too.
A server block with enabled false was skipped by validation, so a typo
in a block written dark surfaced only on the day it was switched on. All
blocks are shape-checked now. Configured static headers carry the bearer
token a real remote server needs, and host_interval bounds how fast one
endpoint is polled.
Found in review of #70.
Second half of the MCP client: the tools the manager discovers become rows in
the existing act allowlist instead of a parallel capability system.
An MCP tool is encoded in the columns that already exist — cmd
["mcp",<server>,<tool>], scope mcp:<server> — so no migration, and
ProposeTool/EnableTool/DisableTool, tool.Matcher and the confirm turn need no
changes. One branch in Executor.Exec routes such a row to the manager instead
of exec, and "mcp" is never run as a binary.
Discovery only ever PROPOSES. destructive comes from the inverse of the MCP
readOnlyHint, so a tool that does not promise to be read-only inherits the
confirm turn, and enabling stays on /tools behind step-up.
Voice args are positional and MCP args are named, so CallPositional binds only
what it can defend: no required properties runs bare, and a read-only tool with
exactly one required string or number gets the tail. Everything else refuses
with ErrNeedsArgs rather than guessing. The read-only condition was learned
against the live Vikunja server: update_task requires only task_id and takes
the rest as optional, so one guessed argument blanked the fields it did not
mention. A partially-filled write destroys what it omits, so a mutating tool
never receives a guessed argument.
Also: a read-only mcp_servers IPC method and an "MCP servers" card on /tools
showing transport, target and state, with the trust level of a local target
spelled out. There is deliberately no call-a-tool IPC method and no run button,
so mutation keeps exactly one path.
Vikunja #251
docs/plans/06-mcp-support.md asks for the host direction — Maven connects OUT
to MCP servers and consumes what they offer. This is the client half: the
protocol, the transports, the connection manager, the config block. Nothing is
wired into a turn yet, and nothing here exposes Maven's own capabilities to an
outside caller.
internal/mcp:
- hand-rolled JSON-RPC 2.0 (the wire format is four fields, and the repo
vendors its deps, so a library would cost more than it saves);
- two transports: a stdio subprocess on this box, and streamable HTTP, which
accepts a plain JSON reply or an SSE frame because servers disagree about
which they send;
- Client: initialize handshake, tools/list, tools/call, resources/list,
resources/read. Text content only — everything downstream is a sentence;
- Manager: lazy dial, per-server failure that never blocks boot or the other
servers, backoff reconnect, Status for a web surface, graceful Close;
- the allowlist encoding: a discovered tool becomes the store row
"vikunja_list_tasks" with cmd ["mcp","vikunja","list_tasks"], scope
"mcp:vikunja". No new column, no migration, and ProposeTool, EnableTool,
the act matcher and the confirm turn all keep working untouched.
Constraints held, in code rather than in prose:
- OFF unless configured, and a server is dark until "enabled": true.
- A url server goes through internal/webfetch, so the SSRF guard, the size
cap, the redirect cap and the per-host rate limit apply. Reaching loopback
needs allow_private on THAT server, and each server gets its own fetcher so
one loopback exemption cannot become a hole for a public endpoint.
- readOnlyHint decides destructive: no hint means "assume it mutates", which
will route the call through the existing confirm turn. Guessing wrong in
that direction only costs a question.
- The catalogue stays small on purpose — allow_tools, and max_tools=12 per
server. The resident model is a 1.7B with a 4096-token context; a tool name
it half-remembers is a wrong act.
- Only the tool name and the router's arguments are sent. There is no API
here through which a note, a fact or the persona block could travel.
webfetch grows Post (JSON-RPC cannot be a GET) and surfaces response headers
for Mcp-Session-Id. It shares Get's guards exactly: a body buys a caller
nothing, a POST to the LAN is refused for the same reason a GET is.
Verified against the real Vikunja MCP server on homesrv
(http://localhost:9100/mcp): handshake, three discovered tools with update_task
correctly NOT read-only, a live list_projects call, a tool excluded by
allow_tools refused, and the same server refused outright once allow_private
was dropped. Tests cover both transports (the stdio one against a real
subprocess), SSE and JSON framing, session echo, reconnect, and the config
validation.
claude
claude