M2 delivers the read-only MCP capability broker gateway and four Day-1 infrastructure adapters (Proxmox, SSH/Linux, GitHub, Gitea). 13 REQs (015-027) all pass. 656 tests green. M1 non-regression verified. MCP spec 2025-06-18 conformance verified (PROTOCOL.md + 7 tests). Defense-in-depth SSH (broker layer 1 + Relay Agent layer 2 + no-shell exec). Two-track LLM smoke (Track A mock-path P0 gate passes). CI: Gitea Actions (.gitea/workflows/ci.yml) with Postgres 16 + RLS verification. Phases shipped: P0 pre-execution v0.1.0 P1 Wave F — MCP gateway v0.1.1 P2 Wave G — Proxmox v0.1.2 P3 Wave H — SSH/Linux v0.1.3 P4 Wave I — Git adapters v0.1.4 P5 Wave J — SSE+smoke+UI v0.1.5 P6 Final — review+ship v0.1.6 ← milestone release ---ci--- phase: 6 milestone: v0.2 status: complete phase_role: final milestone_complete: true requirements: covered: [REQ-015, REQ-016, REQ-017, REQ-018, REQ-019, REQ-020, REQ-021, REQ-022, REQ-023, REQ-024, REQ-025, REQ-026, REQ-027] partial: [] ---/ci---
73 KiB
Research Findings (M2 — MCP Layer & Day 1 Adapters)
Spec: CoreCI Chat v0.1 M2 Engineering Specification v1.0 (.ciagent/steer-m2-spec.md, locked 2026-08-25, Sarah Chen). All 9 open questions resolved; D-001..D-005 carried from M1; D-006 (GitHub fine-grained PAT scopes) and D-007 (MCP transport architecture) recorded in .ciagent/CLARIFY.md.
Date: 2026-08-25
Scope: M2 (REQ-015..027, 13 REQs). MCP capability broker gateway + 4 Day-1 adapters (Proxmox, SSH/Linux via Relay Agent, GitHub, Gitea) + SSE streaming + token-bucket rate limiting + LLM smoke + Postgres 16 CI/RLS verification.
M1 predecessor: M1 research is preserved in git history (commit prior to M2 overwrite). M1 patterns referenced here are grounded in the actual M1 source (apps/relay-agent, apps/control-plane, packages/{db,auth,byom,secrets,config,runtime}).
This document records implementation patterns, pitfalls, and references for each M2 research area so the EXECUTE waves (F..J) have a grounded baseline. Research is scoped to M2; M3 concerns are noted only where they touch M2 boundaries.
R-001 — MCP 2025-06-18 conformance verification (LOWEST CONFIDENCE — most critical)
Scope: The broker implements MCP spec version 2025-06-18. This is the lowest-confidence area (spec §6 gate item 15: "conformance verification artifact — verify before locking"). Verified against modelcontextprotocol.io.
Findings
1. Tool type schema (per 2025-06-18): A tool definition includes:
name(required, string) — unique identifiertitle(optional, string) — NEW in 2025-06-18 — human-readable display name (not present in older spec drafts). The broker SHOULD populatetitlefor the Test-Call UI but it is not required for conformance.description(optional, string) — human-readable descriptioninputSchema(required, JSON Schema) — defines expected parameters. MUST be a JSON Schema object.outputSchema(optional, JSON Schema) — defines expected output structure. NEW in 2025-06-18. If provided, the server MUST return structured results conforming to it, and clients SHOULD validate.annotations(optional, object) — properties describing tool behavior. The spec warns clients MUST treat annotations as untrusted unless from trusted servers.
For M2, the closed tool registry (REQ-015) needs name, description, inputSchema (JSON Schema) — these are the load-bearing required fields. title and outputSchema are optional; M2 MAY use outputSchema for the 9 tools to give the Test-Call UI structured result typing, but it is not required for the gate. annotations can carry the inventory/live classification (M2 spec §5: "inventory capabilities (list_*) get 60s in-memory TTL cache; live capabilities do not") — but since annotations are advisory/untrusted, the broker must NOT rely on them for the cache decision; the broker's own registry metadata (isInventory: boolean) is the authority.
2. tools/list and tools/call JSON-RPC shapes (verified verbatim from spec):
tools/list request (supports pagination via cursor):
{ "jsonrpc": "2.0", "id": 1, "method": "tools/list", "params": { "cursor": "optional-cursor-value" } }
tools/list response:
{ "jsonrpc": "2.0", "id": 1, "result": { "tools": [ { "name": "...", "description": "...", "inputSchema": {...} } ], "nextCursor": "next-page-cursor" } }
tools/call request:
{ "jsonrpc": "2.0", "id": 2, "method": "tools/call", "params": { "name": "get_weather", "arguments": { "location": "New York" } } }
tools/call response (two error mechanisms):
- Protocol errors (unknown tool, invalid args, server errors) → standard JSON-RPC error object:
{ "error": { "code": -32602, "message": "Unknown tool: ..." } }. JSON-RPC error codes: -32700 parse error, -32600 invalid request, -32601 method not found, -32602 invalid params, -32603 internal error. - Tool execution errors (API failures, business logic) → normal result with
isError: true:
{ "jsonrpc": "2.0", "id": 4, "result": { "content": [ { "type": "text", "text": "Failed to fetch: rate limit exceeded" } ], "isError": true } }
- Successful result →
content[]array of content items (text/image/audio/resource_link/resource) +isError: false+ optionalstructuredContent(JSON object, whenoutputSchemaprovided).
M2 mapping: The broker's adapter results map to MCP content[] as a single TextContent block ({type:"text", text: JSON.stringify(normalizedResult)}) for M2. isError is the load-bearing flag the OpenAI translator consumes. The broker does NOT use structuredContent in M2 (the REST facade returns JSON; MCP content[].text carries the serialized JSON). This is a deliberate simplification — M3 may add structuredContent if the chat UI needs typed results.
3. Custom transport requirements (verbatim from spec, §Transports → Custom Transports):
"Clients and servers MAY implement additional custom transport mechanisms to suit their specific needs. The protocol is transport-agnostic and can be implemented over any communication channel that supports bidirectional message exchange. Implementers who choose to support custom transports MUST ensure they preserve the JSON-RPC message format and lifecycle requirements defined by MCP. Custom transports SHOULD document their specific connection establishment and message exchange patterns to aid interoperability."
M2 implication: The in-process custom transport (D-007) is spec-compliant IF and ONLY IF it preserves:
- JSON-RPC 2.0 message format —
tools/listandtools/callrequests/responses MUST be valid JSON-RPC 2.0 envelopes ({jsonrpc:"2.0", id, method, params}/{jsonrpc:"2.0", id, result|error}). The in-process transport passes these as JS objects (no wire serialization needed in-process, but the SHAPE must match). - Lifecycle requirements — initialization (capability negotiation + version agreement), operation, shutdown. The M2 broker↔adapter in-process transport must implement a synthetic
initialize/initializedhandshake on adapter registration, OR document that single-process adapters skip lifecycle because they share the broker process. Recommendation: implement a lightweight syntheticinitializeexchange at adapter registration (broker sends{method:"initialize", params:{protocolVersion:"2025-06-18", capabilities:{tools:{listChanged:false}}}}, adapter responds with{capabilities:{tools:{}}}) so the conformance artifact can point to a real lifecycle exchange. This is cheap and removes the lowest-confidence risk.
4. Streamable HTTP transport — does M2 need it? (verified: NO, the REST facade + SSE is compliant):
The Streamable HTTP transport is a specific standard transport with mandatory behaviors: a single MCP endpoint supporting POST + GET, Accept: application/json, text/event-stream, Mcp-Session-Id header, MCP-Protocol-Version header, session management, resumability via Last-Event-ID. M2's broker↔UI uses a REST facade (GET /api/mcp/tools, POST /api/mcp/invoke, GET /api/mcp/stream/:correlationId) + SSE — this is NOT the MCP Streamable HTTP transport, and that is compliant because:
- The MCP spec defines Streamable HTTP as a standard transport for client-server MCP communication. The broker↔UI is NOT an MCP client-server link — the UI is a browser client of a REST facade. The MCP conformance boundary is broker↔adapter (in-process custom transport) and broker↔CI LLM smoke (stdio). The REST facade is an application-layer convenience that wraps MCP-compliant tool schemas/results for browser consumption. The spec explicitly allows custom transports; a REST facade that carries MCP-shaped payloads is a custom transport pattern.
- Pitfall to document: the REST facade MUST return tool schemas that are MCP
tools/list-shaped ({name, description, inputSchema}) and tool results that are MCPtools/call-result-shaped ({content:[{type:"text",text}], isError}) inside the REST/SSE envelope. ThePOST /api/mcp/invoke→{correlationId, streamUrl}→GET /api/mcp/stream/:correlationIdflow returns SSE events whosedatafield contains the MCP result. This preserves MCP shape at the payload layer while using REST/SSE at the transport layer.
5. OpenAI ↔ MCP translation contract (verified against both specs):
OpenAI Chat Completions tool_calls → MCP tools/call:
- OpenAI:
choices[0].message.tool_calls[i] = { id, type:"function", function: { name, arguments } }whereargumentsis a JSON string. - MCP:
{ method:"tools/call", params: { name, arguments } }whereargumentsis a parsed JSON object. - Translation:
tool_calls[i].function.name→params.name;JSON.parse(tool_calls[i].function.arguments)→params.arguments. Pitfall: OpenAI sendsargumentsas a string; MCP expects an object. The translator MUSTJSON.parseand handle parse failures as a protocol error (not a tool execution error).
MCP tools/call result → OpenAI tool message:
- MCP:
{ content: [{type:"text", text:"..."}], isError: false }(orisError: true). - OpenAI: a follow-up
messages[]entry{ role:"tool", tool_call_id, content }wherecontentis a string. If the LLM should treat it as an error, OpenAI has no nativeisError— the convention is to put the error text incontentand let the LLM read it, OR to surface an error response to the orchestrator. For M2's translator module (packages/mcp/translator.ts):isError: false→{ role:"tool", tool_call_id: <original tool_call id>, content: result.content[0].text }(concatenate text blocks if multiple).isError: true→{ role:"tool", tool_call_id, content: "ERROR: " + result.content[0].text }. The M3 orchestrator decides whether to retry or surface to the user. M2 decision: the translator passesisErrorthrough as a prefix in the content string; the LLM smoke asserts the mock LLM can read it. (Confidence 0.80 — OpenAI has no canonical error-in-tool-message format; this is a reasonable convention.)
6. Conformance verification artifact (M2 gate item 15):
The M2 gate must produce recorded evidence that the broker's MCP implementation conforms to 2025-06-18. Concrete artifact: a tests/mcp-conformance/ directory with:
tools-list.test.ts— assertsGET /api/mcp/toolsreturns an array where each tool has{name, description, inputSchema}(JSON Schema object withtype:"object"), and the 9-tool closed set matches REQ-015 exactly. Snapshot the fulltools/listresponse.tools-call-happy.test.ts— invokes a mock adapter viaPOST /api/mcp/invoke, asserts the SSE stream emits atool_resultevent whosedataparses to{content:[{type:"text",text}], isError:false}— the MCP result shape.tools-call-error.test.ts— invokes a mock adapter that returnsisError:true, asserts the SSEerrorterminal event carries the MCP error shape.tools-call-invalid-args.test.ts— invokes a tool with args not matchinginputSchema, asserts HTTP 400 with a schema-validation error (broker rejects before adapter invocation — this is a protocol error, mapped to JSON-RPC error shape internally even though the REST facade returns HTTP 400).translator.test.ts— asserts the OpenAI↔MCP translator:tool_calls[].function.{name, arguments(JSON string)}→params.{name, arguments(object)}andresult.content[].text + isError→ OpenAI{role:"tool", tool_call_id, content}.lifecycle.test.ts— asserts the in-process custom transport performs the syntheticinitialize/initializedhandshake on adapter registration and preserves JSON-RPC 2.0 envelope shape.- Spec-version pin: a constant
MCP_PROTOCOL_VERSION = "2025-06-18"exported frompackages/mcpand asserted in the conformance test header. A comment linking tohttps://modelcontextprotocol.io/specification/2025-06-18/server/toolsand.../basic/transports.
The artifact is a passing test suite + a CONFORMANCE.md note documenting: (a) the spec version, (b) which transports are used (in-process custom, stdio for LLM smoke, REST facade + SSE for UI — NOT Streamable HTTP), (c) the JSON-RPC shapes preserved, (d) the OpenAI↔MCP translation contract, (e) the synthetic lifecycle handshake. This satisfies gate item 15.
Pitfalls
titleandoutputSchemaare new in 2025-06-18 — older references show the schema without them. Pin to2025-06-18and document the version in code.argumentstype mismatch — OpenAI string vs MCP object. The translator MUST parse.isErroris MCP-specific — OpenAI has no equivalent; the translator convention (prefix "ERROR:") is an M2 decision, not spec-mandated.- Streamable HTTP is NOT required — the REST facade + SSE is a compliant custom transport pattern, but the broker must document this. Do NOT implement
Mcp-Session-IdorMCP-Protocol-VersionHTTP headers on the REST facade (those are Streamable HTTP transport specifics). - Pagination —
tools/listsupportscursor. M2's closed 9-tool set is small enough to return in one page (nonextCursor); the broker should omitnextCursorwhen there are no more pages.
References
- MCP Tools spec: https://modelcontextprotocol.io/specification/2025-06-18/server/tools
- MCP Transports spec: https://modelcontextprotocol.io/specification/2025-06-18/basic/transports
- MCP Lifecycle spec: https://modelcontextprotocol.io/specification/2025-06-18/basic/lifecycle
- JSON-RPC 2.0: https://www.jsonrpc.org/specification
Confidence: 0.80 (spec verified verbatim; the only residual risk is the synthetic lifecycle handshake decision for in-process adapters — documented as a recommendation, not a spec mandate).
R-002 — Proxmox VE API client patterns (PVEAuditor, read-only)
Scope: REQ-020 (Proxmox adapter), REQ-025 (PVEAuditor token auth). 3 capabilities: proxmox.list_vms, proxmox.get_vm_status, proxmox.get_node_metrics.
Findings
1. PVE API authentication — two modes:
-
Ticket/Cookie auth: POST
https://host:8006/api2/json/access/ticketwithusername=root@pam&password=...→ returns{data:{ticket, CSRFPreventionToken, username}}. Theticketis set as cookiePVEAuthCookie=<ticket>. Write requests (POST/PUT/DELETE) require theCSRFPreventionTokenheader. Tickets expire in 2 hours. NOT used by M2 — M2 uses API tokens (stateless, no password handling, no 2h expiry). -
API Token auth (M2 uses this): Set HTTP
Authorizationheader toPVEAPIToken=USER@REALM!TOKENID=UUID. Example:Authorization: PVEAPIToken=root@pam!monitoring=aaaaaaaaa-bbbb-cccc-dddd-ef0123456789. Tokens do NOT need CSRF tokens for POST/PUT/DELETE (the CSRF attack vector doesn't apply to non-browser token clients). Tokens are stateless and have separate permissions + expiration. This is exactly the M2 pattern: the operator creates a token scoped toPVEAuditorrole, the broker stores the fullPVEAPIToken=...string via SecretProvider, and the adapter sends it as theAuthorizationheader on every GET request.
PVEAuditor role scoping: PVE has built-in roles; PVEAuditor is a read-only role that grants Datastore.Audit, Sys.Audit, VM.Audit, etc. — sufficient to GET nodes, qemu, and status endpoints. The token is created under a user (e.g. root@pam or a dedicated service user) with the token's permission boundary set to PVEAuditor at the / path (or a specific node path). The token inherits the user's role but can be further restricted; it CANNOT exceed the user's permissions.
2. The 3 M2 capabilities' upstream endpoints:
| Capability | Method | Endpoint | Notes |
|---|---|---|---|
proxmox.list_vms (inventory) |
GET | /api2/json/nodes/{node}/qemu |
List VMs on a node. Requires VM.Audit. Returns {data: [{vmid, name, status, ...}]}. Pitfall: requires a node argument — list_vms must first call GET /api2/json/nodes to enumerate nodes, then call /qemu per node, OR the broker's list_vms inputSchema requires a node param. Recommendation: list_vms inputSchema requires node (string); the Test-Call UI fetches the node list first via a separate list_nodes-like call OR list_vms returns VMs across all nodes by calling /nodes then /qemu per node. Simplest M2: list_vms requires node arg; a future list_nodes tool (not in M2's 9) would populate the picker. For M2, the adapter config can store a default node, OR the UI shows a node input. Decision: list_vms inputSchema: {node: string (required)} — keep it simple; the operator knows their node names. |
proxmox.get_vm_status (live) |
GET | /api2/json/nodes/{node}/qemu/{vmid}/status/current |
Current VM status. Requires VM.Audit. Returns {data: {vmid, status, cpu, mem, ...}}. inputSchema: {node: string, vmid: integer}. |
proxmox.get_node_metrics (live) |
GET | /api2/json/nodes/{node}/status |
Node status/metrics. Requires Sys.Audit. Returns {data: {cpu, memory, uptime, ...}}. inputSchema: {node: string}. |
Pitfall: the spec REQ-020 also mentions GET /api2/json/nodes and GET /api2/json/qemu — note that /api2/json/qemu is NOT a valid PVE endpoint (qemu is under a node). The 3 valid endpoints are /nodes, /nodes/{node}/qemu, /nodes/{node}/qemu/{vmid}/status/current, /nodes/{node}/status. The M2 adapter calls ONLY these GETs.
3. Rate limiting in PVE API: Proxmox VE does NOT document a hard rate limit, but the API is served by pveproxy (a Perl HTTP daemon) which has connection limits. Heavy polling can degrade the node. The M2 broker's token-bucket (60 user/min, 300 tenant/min) plus the 60s inventory cache for list_vms is sufficient backstop. No Retry-After header from PVE. Pitfall: if a customer's PVE is under load, the 10s upstream timeout (NFR) may fire → HTTP 504. The adapter should treat 5xx from PVE as a transient upstream error (HTTP 502/504 to the caller), not a write rejection.
4. TypeScript HTTP client patterns for PVE:
- Base URL:
https://<host>:8006/api2/json/(port 8006, HTTPS, often self-signed in customer labs). - Pitfall — TLS: customer PVE hosts frequently use self-signed certs. The adapter MUST allow
rejectUnauthorized: falsefor PVE specifically (configurable per-adapter; default true in prod, but aallowSelfSignedconfig flag for PVE since it's the common case). Security note: this is per-adapter config, stored in themcp_adapters.configJSON column, NOT a global setting. The broker validates it's only set for Proxmox adapters. - No cookie handling needed for token auth — just the
Authorizationheader on every request. - Use the global
fetch(Node 18+) withAbortSignal.timeout(10_000)for the 10s upstream NFR (mirrorspackages/byom/validator.tspattern). - Response shape:
{ data: <payload> }— the adapter unwrapsdata. Errors: PVE returns{ data: null, errors: "..." }with HTTP 5xx, or HTTP 200 with{data: null}for some not-found cases. Pitfall: check both!res.okANDbody.data === null.
5. PVE 7.x vs 8.x API differences: The API is "API stable within a major release." The endpoints M2 uses (/nodes, /nodes/{node}/qemu, /nodes/{node}/qemu/{vmid}/status/current, /nodes/{node}/status) are unchanged from PVE 6.x through 8.x. The base path /api2/json/ is stable. No version-detection needed for PVE (unlike Gitea). The adapter records the PVE version (from GET /api2/json/version during test_connection) in the config row for diagnostics, but does not branch on it. Confidence: 0.85 — these endpoints are core and have not changed.
PVEAuditor validation at submit time (REQ-025): When Sam submits a Proxmox adapter config, the broker must verify the token's role is PVEAuditor before persisting. Approach: call GET /api2/json/access/users/{user}/token/{tokenid} with the token — this returns the token's permissions. Pitfall: introspecting the token's role is non-trivial; PVE doesn't have a clean "what role does this token have" endpoint. Practical approach: call GET /api2/json/version (any valid token can call this) to verify the token is valid, then attempt a read-only audit call like GET /api2/json/nodes — if it succeeds, the token has at least Sys.Audit; if a write-capable call would be needed to verify PVEAuditor specifically, that's a gap. M2 decision: the broker verifies the token is valid (GET /version succeeds) AND that a read-only audit call succeeds (GET /nodes returns 200). True PVEAuditor role enforcement is the operator's responsibility at token creation time (documented in the UI: "Create a token with PVEAuditor role"). The broker's submit-time check is "token works for reads," not "token lacks writes" — because PVE has no introspection for "does this token have write perms." The write-method blocklist (REQ-018: reject POST/PUT/DELETE at broker) is the load-bearing safety boundary. Confidence: 0.70 — this is a pragmatic validation; true role introspection is a PVE gap. Document this in the adapter config UI help text.
Implementation pattern
// packages/mcp/adapters/proxmox/client.ts (sketch)
async function pveGet(host: string, token: string, path: string, allowSelfSigned: boolean): Promise<unknown> {
const url = `https://${host}:8006/api2/json${path}`;
const agent = allowSelfSigned ? new https.Agent({ rejectUnauthorized: false }) : undefined;
const res = await fetch(url, {
headers: { Authorization: `PVEAPIToken=${token}` },
signal: AbortSignal.timeout(10_000),
// @ts-expect-error Node fetch agent
agent,
});
if (!res.ok) throw new PveUpstreamError(`PVE ${res.status}: ${await res.text()}`);
const body = await res.json() as { data: unknown };
if (body.data === null) throw new PveUpstreamError(`PVE: null data for ${path}`);
return body.data;
}
References
- Proxmox VE API: https://pve.proxmox.com/wiki/Proxmox_VE_API
- PVE API viewer: https://pve.proxmox.com/pve-docs/api-viewer/index.html
- API Tokens section (PVEAPIToken format, no CSRF for tokens)
Confidence: 0.80 (API verified; PVEAuditor introspection is the residual risk, documented as a pragmatic decision).
R-003 — SSH adapter via M1 Relay Agent (defense-in-depth, WebSocket)
Scope: REQ-021 (SSH/Linux adapter via Relay Agent), REQ-026 (SSH key + whitelist execution). One capability: ssh.run_whitelisted_command.
Findings (grounded in M1 source: apps/relay-agent/)
1. M1 Relay Agent WebSocket protocol (verified from apps/relay-agent/wsclient/client.go):
- Endpoint:
wss://<saas>/api/relay/ws(control-planews-server.tshandles upgrade). - Auth:
Authorization: Bearer <tenantToken>on handshake (JWT relay registration token, verified viaverifyRelayToken). - M1 message types implemented:
register(agent→server),registered(server→agent),ping(agent→server heartbeat every 30s),pong(server→agent). Unknown message types get an{type:"error", error:"unknown message type"}response. - M1 explicitly leaves extensibility for M2: the Go reader goroutine comment says "Not a pong; ignore but keep the loop alive for protocol extensibility (M2 tool-call messages will arrive here)." The M1 server
handleMessageswitch has adefaultcase that returns an error for unknown types. M2 adds atool_callmessage type — the control-planews-server.tsmust add atool_callhandler, and the Go agent's reader goroutine must routetool_callmessages to a new execution path.
2. M1 CheckCommand whitelist hook (verified from apps/relay-agent/whitelist/whitelist.go):
- Signature:
CheckCommand(cmd string) error— THIS IS THE LOCKED G-004 CONTRACT. M2's SSH adapter calls this BEFORE constructingexec.Command. Any change requires a documented migration. - The whitelist JSON (
ssh-whitelist.json) is versioned (version: 1) withcommands(allowed command prefixes) andarguments.deny(forbidden tokens like-exec,|,>,&&). - M1's whitelist is BROADER than M2's 6-command subset. M1 ships:
cat, ls, systemctl status, journalctl, df, du, ps, top, ss, netstat, ip, uptime, uname, free, who, w, last, dmesg, lscpu, lspci, lsblk, mount, findmnt, hostname, ip addr, ip route, ss -tlnp. - M2's 6-command subset (spec §7 Q3):
uptime,df -h,free -m,systemctl status <svc>,journalctl -n <N>(1-500),systemctl list-units --type=service. This is a SUBSET of M1's whitelist. M2's broker validates against this 6-command subset (layer 1, BEFORE dispatch); M1'sCheckCommandvalidates against the broader M1 whitelist (layer 2, at execution). Both must pass. Layer 1 is stricter (6 commands) than layer 2 (M1's full whitelist) — this is correct defense-in-depth: the broker rejects anything outside the 6, and the Relay Agent would reject anything outside M1's broader set even if the broker were bypassed.
3. Broker-side validation (layer 1) — how to validate the 6-command subset:
The 6 commands are structured, not free-form. The broker must parse the command argument and validate it matches one of:
uptime— exact match (no args).df -h— exact match.free -m— exact match.systemctl status <svc>— prefixsystemctl statusfollowed by a service name (alphanumeric +-+_+.). Pitfall:<svc>is user-supplied; the broker must sanitize (regex^[a-zA-Z0-9_.-]+$, max 64 chars) to prevent injection likesystemctl status nginx; rm -rf /.journalctl -n <N>—journalctl -nfollowed by an integer 1-500. Regex^journalctl -n ([1-9][0-9]{0,2}|500)$.systemctl list-units --type=service— exact match.
Implementation pattern: a validateSshCommand(command: string): {ok: boolean, reason?: string} function in packages/mcp/adapters/ssh/whitelist.ts using a small rules table. Do NOT use the M1 Go whitelist's tokenization — that's Go and runs in the agent. The broker (TypeScript) reimplements the 6-command validation independently (defense-in-depth: two independent implementations). Pitfall: the broker validation and the Go CheckCommand are deliberately independent codepaths so a bug in one doesn't bypass the other.
4. WebSocket message format for tool_call (M2 addition to the M1 protocol):
The M1 protocol uses JSON messages over the WebSocket. M2 adds:
- Server→Agent:
{ "type": "tool_call", "callId": "<ulid>", "command": "uptime", "timeoutMs": 10000 } - Agent→Server (success):
{ "type": "tool_result", "callId": "<ulid>", "stdout": "...", "stderr": "...", "exitCode": 0 } - Agent→Server (error/rejection):
{ "type": "tool_result", "callId": "<ulid>", "error": "whitelist rejected: ...", "exitCode": -1 }(the Relay Agent'sCheckCommandrejection path). - Agent→Server (timeout):
{ "type": "tool_result", "callId": "<ulid>", "error": "timeout after 10s", "exitCode": -1 }.
The Go agent's reader goroutine (currently only handles pong) must route tool_call messages to a new executor goroutine that: (a) calls CheckCommand(command) — if error, return tool_result with the rejection; (b) runs exec.Command with a 10s context timeout; (c) returns tool_result with stdout/stderr/exitCode. The Go exec.Command must use the parsed argv, NOT a shell — exec.Command("systemctl", "status", "nginx"), never sh -c "...". This is the third enforcement layer (no shell injection).
5. target_id routing: Each Relay Agent registers as one target (M1 handleRegister inserts into targets and returns targetId). The control-plane ws-server.ts tracks connectedAgents: Map<WebSocket, ConnectedAgent> keyed by WebSocket. M2 needs a reverse index targetsByTenant: Map<tenantId, Map<targetId, WebSocket>> so the broker can route ssh.run_whitelisted_command with a target_id to the correct WebSocket. Pitfall: if the target is disconnected (agent offline), the broker returns HTTP 404 with a structured error (REQ-016 routing error) — do NOT queue the call. Pitfall: the broker must check the target belongs to the same tenant (RLS — withTenant + the targets table tenant_id).
6. Timeout handling (10s upstream NFR): The broker wraps the WebSocket tool_call → tool_result round-trip in a 10s timeout (AbortController on the broker side). If the agent doesn't respond in 10s, the broker emits an SSE error terminal event with "upstream timeout" and HTTP 504 semantics. The Go agent independently enforces a 10s exec.Command timeout so a hung command doesn't hold the WebSocket. Both timeouts must be 10s — if they differ, the broker should time out first (so the SSE stream closes cleanly) — set broker timeout to 10s and agent exec timeout to 9.5s (agent returns timeout result before broker gives up). Confidence: 0.85.
Pitfalls
- Two independent whitelist implementations (TS broker, Go agent) — keep them in sync semantically but independent in code.
- No shell in the Go executor —
exec.Commandwith split argv. systemctl status <svc>service name injection — broker must regex-validate.journalctl -n <N>range — 1-500 only.- Target offline → HTTP 404, not a queue.
- M1 non-regression: the M2
tool_callmessage type is additive; M1'sregister/ping/pongmust continue to work. The Go agent's reader goroutine change must not break the heartbeat loop.
References
- M1 source:
apps/relay-agent/wsclient/client.go,apps/relay-agent/whitelist/whitelist.go,apps/relay-agent/whitelist/ssh-whitelist.json,apps/control-plane/ws-server.ts
Confidence: 0.85 (grounded in actual M1 code; the tool_call message addition is a clean extension of the existing protocol).
R-004 — GitHub REST API adapter (fine-grained PAT, D-006)
Scope: REQ-022 (GitHub adapter), REQ-027 (scoped token auth). 3 capabilities: github.list_repos, github.get_recent_ci_runs, github.get_workflow_run. D-006: fine-grained PAT with metadata:read + actions:read minimum (no contents:read).
Findings (verified from GitHub REST API docs)
1. GET /user/repos?per_page=100 — list repos for the authenticated user:
- Auth:
Authorization: Bearer <token>,Accept: application/vnd.github+json,X-GitHub-Api-Version: 2022-11-28(the docs show2026-03-10as the latest, but2022-11-28is the stable GA version; M2 should use2022-11-28for stability). - Response 200: array of
Minimal Repositoryobjects —id,name,full_name,owner.login,private,description,html_url,default_branch,updated_at, etc. (very large objects; the adapter normalizes to{id, name, full_name, owner, private, description, html_url, default_branch, updated_at}). - Pagination:
per_page(max 100),page(default 1). TheLinkheader containsnext/prevURLs. M2 decision:github.list_repos(inventory, 60s cache) fetchesper_page=100and followsLinknext until exhausted OR a reasonable cap (e.g., 500 repos = 5 pages) to bound latency. inputSchema:{per_page?: integer (default 100, max 100), page?: integer (default 1)}— but the broker should auto-paginate for the inventory call and return a flat list. Simpler M2:list_repostakes no args, returns up to 100 repos (first page,per_page=100). If the tenant has >100 repos, the UI shows "first 100" and a note. Multi-page is an M3 enhancement. Confidence: 0.80 — keeps M2 simple.
2. GET /repos/{owner}/{repo}/actions/runs?per_page={limit} — list Actions runs:
- Response 200:
{ total_count: integer, workflow_runs: [WorkflowRun] }. EachWorkflowRun:id,name,head_branch,head_sha,event,status,conclusion,workflow_id,html_url,created_at,updated_at,run_number,actor.login,repository(embedded minimal repo).statusis one ofcompleted|in_progress|queued|...;conclusionis one ofsuccess|failure|cancelled|neutral|skipped|timed_out|...(null while in_progress). - inputSchema for
github.get_recent_ci_runs:{owner: string, repo: string, per_page?: integer (default 30, max 100), status?: string, branch?: string}. The adapter normalizes to{total_count, runs: [{id, head_branch, status, conclusion, html_url, created_at, actor}]}. - Pitfall:
ownerandrepoare case-insensitive per the API, but the broker should preserve the user's casing for display.
3. GET /repos/{owner}/{repo}/actions/runs/{run_id} — get a single workflow run:
- Response 200: a single
WorkflowRunobject (same shape as array elements above). inputSchema:{owner, repo, run_id: integer}.
4. Fine-grained PAT scope validation (D-006 — THE CRITICAL FINDING):
GitHub does NOT provide a public API endpoint to introspect a fine-grained PAT's granted scopes at runtime. Classic PATs expose X-OAuth-Scopes header on GET /user (e.g., repo, read:org), but fine-grained PATs do NOT return their permission list via any API response header or body. The permissions are encoded in the token's signed payload and validated server-side per-request.
However, GitHub DOES return the X-Accepted-GitHub-Permissions header on responses — this header tells you what permissions the endpoint required (e.g., metadata=read, actions=read), which helps diagnose 403s but doesn't list what the token has.
M2 broker submit-time validation strategy (REQ-027, D-006):
- Detect classic vs fine-grained: Classic PATs start with
ghp_(orgho_/ghu_); fine-grained PATs start withgithub_pat_. The broker rejects classic PATs at submit (D-006: fine-grained only — classicreposcope grants write). Pitfall: the token prefix is the discriminator. If the token doesn't start withgithub_pat_, reject with HTTP 422 "fine-grained PAT required." - Validate the token works + has metadata:read: call
GET /userwith the token. If 401 → invalid token (HTTP 422). If 200 → token is valid. All fine-grained PATs requiremetadata:readimplicitly (it's mandatory on every fine-grained PAT), so a successfulGET /userimpliesmetadata:read. - Validate
actions:read: callGET /user/repos?per_page=1— wait, this requiresmetadata:read(which we have). To validateactions:readspecifically, attemptGET /repos/{any-repo}/actions/runs?per_page=1— but we don't know a repo yet at submit time. Practical approach: the broker stores the token as "validated for metadata" at submit (GET /user succeeded), and validatesactions:readat invocation time per-tool: whengithub.get_recent_ci_runsorgithub.get_workflow_runis called, if GitHub returns 403 withX-Accepted-GitHub-Permissionsindicatingactions=readwas required, the broker surfaces HTTP 403 +adapter.write_rejectedaudit event... NO — a 403 for missingactions:readis a scope-mismatch, not a write attempt. Refinement: the broker distinguishes: (a) 403 from GitHub for missing scope → HTTP 403 "insufficient scope" + auditadapter.capability_invokedwithresult=failure(NOTwrite_rejected— no write was attempted); (b) the broker's own write-method blocklist (rejecting POST/PUT/DELETE) →adapter.write_rejected. These are different. - Submit-time best effort: call
GET /user(validates token + implicit metadata:read). Recordvalidated=true. Theactions:readis validated on firstget_recent_ci_runs/get_workflow_runinvocation. The UI help text says "ensure the PAT hasactions:read." This is the pragmatic M2 approach since GitHub offers no fine-grained scope introspection. Confidence: 0.75 — this is a known GitHub gap; the broker cannot do better without GitHub adding a scope introspection endpoint.
5. Rate limiting (verified):
- Authenticated primary rate limit: 5,000 req/hour per token.
- Headers:
x-ratelimit-limit,x-ratelimit-remaining,x-ratelimit-used,x-ratelimit-reset(UTC epoch seconds). - Exceeding → HTTP 403 or 429 with
x-ratelimit-remaining: 0. Retry afterx-ratelimit-reset. - Secondary rate limits: 100 concurrent, 900 points/min (GET=1pt, POST=5pt). Exceeding → 403/429 with
retry-afterheader. - M2 adapter behavior: observe
x-ratelimit-remaining; if it hits 0, do NOT make the call — return HTTP 429 to the caller withRetry-After: <seconds until x-ratelimit-reset>. If GitHub returns 429, back off exponentially (1s, 2s, 4s, max 3 retries) then surface 429 to the caller. The M2 broker's own token-bucket (60/min user) is well below GitHub's 5000/hour, so the GitHub limit is unlikely to bind unless many tenants share a token (they shouldn't — per-tenant tokens).
Implementation pattern
// packages/mcp/adapters/github/client.ts (sketch)
async function ghGet(path: string, token: string, query?: Record<string,string>): Promise<unknown> {
const url = new URL(`https://api.github.com${path}`);
for (const [k,v] of Object.entries(query ?? {})) url.searchParams.set(k, v);
const res = await fetch(url, {
headers: { Authorization: `Bearer ${token}`, Accept: "application/vnd.github+json", "X-GitHub-Api-Version": "2022-11-28" },
signal: AbortSignal.timeout(10_000),
});
if (res.status === 429 || (res.status === 403 && res.headers.get("x-ratelimit-remaining") === "0")) {
const reset = Number(res.headers.get("x-ratelimit-reset") ?? 0);
const retryAfter = Math.max(1, reset - Math.floor(Date.now()/1000));
throw new GithubRateLimitError(retryAfter);
}
if (res.status === 403) {
const accepted = res.headers.get("x-accepted-github-permissions") ?? "";
throw new GithubScopeError(`403; required permissions: ${accepted}`);
}
if (!res.ok) throw new GithubUpstreamError(`GitHub ${res.status}: ${await res.text()}`);
return res.json();
}
Pitfalls
- No fine-grained scope introspection — the M2 broker does best-effort (GET /user) + per-invocation 403 handling.
- Classic PAT rejection —
github_pat_prefix check at submit. X-GitHub-Api-Version— pin to2022-11-28(stable GA).- Large response objects — normalize to a subset to keep SSE payloads small.
- 429 vs 403-with-ratelimit — GitHub uses both; check
x-ratelimit-remaining.
References
- GitHub repos API: https://docs.github.com/en/rest/repos/repos
- GitHub Actions workflow runs: https://docs.github.com/en/rest/actions/workflow-runs
- GitHub users API: https://docs.github.com/en/rest/users/users
- GitHub rate limits: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
- Fine-grained PAT permissions: https://docs.github.com/en/rest/authentication/permissions-required-for-fine-grained-personal-access-tokens
- PAT management (token prefixes, fine-grained vs classic): https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/creating-a-personal-access-token
Confidence: 0.78 (API verified; the fine-grained scope introspection gap is the residual risk, documented with the pragmatic mitigation).
R-005 — Gitea REST API adapter (version-aware, D-007)
Scope: REQ-023 (Gitea adapter), REQ-027 (scoped token auth). 2 capabilities: gitea.list_repos, gitea.get_recent_ci_runs. Version-aware: ≥1.22 requires read:repository; <1.22 accepts any token with broker-side write-method blocklist.
Findings
1. GET /api/v1/version — version detection:
- Auth: none required (public endpoint). Response:
{ version: "1.22.0", revision: "...", commit: "..." }. - The broker parses
versionand compares to1.22. Pitfall: Gitea versions are semver-ish (1.22.0,1.22.1,1.23.0); compare major.minor:>= 1.22. Record the version in themcp_adapters.configJSON column at submit time.
2. GET /api/v1/user/repos?limit=50 — list repos:
- Auth:
Authorization: token <token>(Gitea usestokennotBearer). Response: array of repo objects —id,name,full_name,owner.login,private,description,html_url,default_branch,updated_at. Mirrors GitHub shape closely (Gitea's API is GitHub-inspired). - Pagination:
limit(default 50, max 50 in Gitea),page(default 1). M2gitea.list_reposreturns up to 50 (first page). inputSchema:{}(no args; auto-paginate or first-page only).
3. GET /api/v1/repos/{owner}/{repo}/actions/runs?limit={limit} — list Actions runs (Gitea Actions, added in Gitea 1.19+):
- Gitea Actions is GitHub Actions-compatible; the API mirrors GitHub's shape:
{ total_count, workflow_runs: [{id, head_branch, status, conclusion, html_url, created_at, ...}] }. - Pitfall: Gitea Actions requires the feature to be enabled on the Gitea instance (
actions.ENABLED=truein app.ini). If disabled, this endpoint returns 404. The adapter should surface 404 as "Gitea Actions not enabled on this instance" (HTTP 502 to caller, not a write rejection). - inputSchema:
{owner, repo, limit?: integer (default 30, max 50)}.
4. Gitea ≥1.22 fine-grained OAuth2 scopes (read:repository):
Gitea 1.22 added fine-grained OAuth2 token scopes (modeled on GitHub's fine-grained PATs). Scopes include read:repository, write:repository, read:issue, etc. A token with read:repository can list repos and read repo metadata. Pitfall: Gitea's scope system applies to OAuth2 tokens; plain API tokens (created via user settings) may not carry scopes the same way. The M2 broker validates at submit time by calling GET /api/v1/user/repos?limit=1 — if it returns 200, the token has read access; if 403, insufficient scope. This is the same pragmatic approach as GitHub (no clean scope introspection; validate by attempting a read).
5. Gitea <1.22 coarse-grained tokens + broker-side write-method blocklist:
Gitea <1.22 has only coarse-grained tokens (no read: scopes). Any valid token can read AND write. The M2 spec §7 Q6 decision: accept any token for Gitea <1.22 with the broker-side write-method blocklist (POST/PUT/DELETE/PATCH on all endpoints) as the security backstop. The broker NEVER sends a non-GET to Gitea, so even an over-scoped token cannot cause a write through the broker. The adapter.write_rejected audit event fires if (somehow) a write method reached the broker — but since the adapter only constructs GET fetches, this is belt-and-suspenders.
6. Submit-time validation (GET /api/v1/repos/search?limit=1 per spec §7 Q6):
The spec says "Submit-time validation via GET /api/v1/repos/search?limit=1." This is a public-ish endpoint that works with any valid token. The broker: (a) calls GET /api/v1/version → parse version; (b) if ≥1.22, calls GET /api/v1/user/repos?limit=1 to confirm read:repository (200 = ok, 403 = insufficient scope → HTTP 422); (c) if <1.22, calls GET /api/v1/repos/search?limit=1 to confirm token validity (200 = ok). Record version + validated flag.
Implementation pattern
Mirror the GitHub adapter (packages/mcp/adapters/gitea/client.ts) with: base URL is the customer's Gitea host (https://gitea.example.com/api/v1/), auth header Authorization: token <token>, version detection at submit, write-method blocklist enforced at broker for all versions.
Pitfalls
Authorization: token <token>notBearer(Gitea quirk).- Gitea Actions may be disabled → 404, surface as "not enabled."
- Version comparison — semver-ish; compare major.minor as integers.
- Self-signed certs — customer Gitea often self-signed; same
allowSelfSignedper-adapter flag as Proxmox. - No
gitea.get_workflow_runin M2 (deferred to v1.2+ per Q2) — onlylist_repos+get_recent_ci_runs.
References
- Gitea API Swagger: https://gitea.com/api/swagger (and
/api/swaggeron any Gitea instance) - Gitea API is auto-documented; the OpenAPI spec is at
/swagger.v1.jsonon any instance.
Confidence: 0.72 (Gitea docs page required JS and didn't fetch cleanly; findings are from the M2 spec + known Gitea API conventions mirroring GitHub. The version-aware scope validation is the residual risk — recommend the implementer verify against a running Gitea 1.22+ and <1.22 instance during Wave I).
R-006 — SSE streaming in Next.js App Router (REQ-017)
Scope: REQ-017 (SSE stream on GET /api/mcp/stream/:correlationId). Per-call streams, ULID correlation IDs.
Findings (grounded in M1 Next.js App Router patterns: apps/control-plane/app/api/byom/route.ts)
1. Route Handler for GET /api/mcp/stream/:correlationId:
Next.js 15 App Router route handlers export GET(req: NextRequest). Dynamic segments use [correlationId]/route.ts. The handler returns a Response with Content-Type: text/event-stream and a ReadableStream body (Node.js ReadableStream web stream).
// apps/control-plane/app/api/mcp/stream/[correlationId]/route.ts (sketch)
export const runtime = "nodejs";
export async function GET(req: NextRequest, { params }: { params: { correlationId: string } }) {
const stream = new ReadableStream({
start(controller) {
const ctx = correlationContext.get(params.correlationId);
if (!ctx) { controller.enqueue(encodeSse("error", { error: "unknown correlation" })); controller.close(); return; }
ctx.controller = controller; // adapter emits events into this
req.signal.addEventListener("abort", () => { ctx.cancel(); correlationContext.delete(params.correlationId); });
},
});
return new Response(stream, { headers: { "Content-Type": "text/event-stream", "Cache-Control": "no-cache", "Connection": "keep-alive" } });
}
Pitfall: runtime = "nodejs" is required (edge runtime can't do long-lived streams with our in-memory Map). Set dynamic = "force-dynamic" to avoid static caching.
2. SSE event format (per M2 spec §5):
id: <ulid>-<seq>\n
event: tool_result\n
data: {"content":[...],"isError":false}\n
\n
Terminal events: event: done (completion) or event: error (failure), then close the stream. The id is <ulid>-<sequence> where ULID is the correlation ID and sequence is a per-stream incrementing integer. Pitfall: SSE fields are newline-separated; the event is terminated by a blank line (\n\n). The data field is a single JSON string on one line (no embedded newlines). Use JSON.stringify once.
3. Client disconnect detection:
Next.js Route Handlers receive req.signal (AbortSignal). When the EventSource (browser) closes, req.signal is aborted. The handler adds req.signal.addEventListener("abort", cleanup). On abort: cancel the in-flight adapter call (AbortController), delete the correlation context entry, do NOT append an audit event (spec Edge 8: "no audit event for client-side cancellation"). Pitfall: the abort may fire after the stream already closed normally — guard with a closed flag.
4. ULID generation:
- Use the
ulidnpm package (ulid()returns a 26-char Crockford-base32 string, lexicographically sortable by time). Add as a dependency topackages/mcp(not the whole control-plane — keep the dependency in the package that mints IDs). - Pitfall: ULIDs are monotonic only if generated in the same process with a monotonic factory; use
ulid()for simplicity in M2 (single process). For distributed generation (M3), usemonotonicFactory(). - The correlation ID format:
01HXXXXXXXXXXXXXXXXXXXXXX(26 chars). The SSEidappends-<seq>:01HXXXXXXXXXXXXXXXXXXXXXX-0,...-1, etc.
5. Correlation context management:
- In-memory
Map<correlationId, CorrelationContext>inpackages/mcp/stream-manager.ts. Each context holds:{ correlationId, tenantId, userId, adapterType, toolName, controller?: ReadableStreamController, abortController: AbortController, createdAt }. POST /api/mcp/invokemints the ULID, creates the context, kicks off the adapter call (async, emits events into the controller), and returns{ correlationId, streamUrl: "/api/mcp/stream/<correlationId>" }.GET /api/mcp/stream/:correlationIdlooks up the context, attaches thereq's ReadableStream controller, and streams events until done/error/abort.- Cleanup on: (a) terminal event (
done/error) → close controller, delete context; (b) client disconnect → cancel adapter, delete context, no audit; (c) timeout safety net → a 60s max-stream lifetime timer deletes orphaned contexts. - Pitfall: the
POST /api/mcp/invokereturns BEFORE the stream is consumed — the adapter call runs concurrently. If the client never opens the SSE stream, the adapter call completes but events are buffered in the controller's internal queue (backpressure). Add a 30s "stream not opened" timeout: ifGET /api/mcp/stream/:correlationIdisn't called within 30s ofPOST /invoke, cancel the adapter call and delete the context.
Implementation pattern
function encodeSse(event: string, data: unknown, id: string): Uint8Array {
const text = `id: ${id}\nevent: ${event}\ndata: ${JSON.stringify(data)}\n\n`;
return new TextEncoder().encode(text);
}
Pitfalls
runtime = "nodejs"mandatory (not edge).force-dynamicto prevent caching.- Backpressure if client is slow — ReadableStream handles this (the controller queues). Cap the queue (e.g., 100 events) and if exceeded, cancel with "client too slow."
- No audit on client cancel (Edge 8) — but DO audit on normal completion and error.
- Stream-not-opened timeout (30s) to prevent orphan adapter calls.
References
- Next.js Route Handlers: https://nextjs.org/docs/app/api-reference/file-conventions/route
- SSE spec: https://html.spec.whatwg.org/multipage/server-sent-events.html
ulidnpm: https://www.npmjs.com/package/ulid- M1 pattern:
apps/control-plane/app/api/byom/route.ts(NextResponse, runtime=nodejs, auth guard)
Confidence: 0.85 (standard Next.js + SSE pattern; M1 route handler pattern confirmed in source).
R-007 — Token-bucket rate limiting in TypeScript (REQ-019)
Scope: REQ-019. Per user (60 req/min, refill 1/sec) + per tenant (300 req/min, refill 5/sec). In-memory, process-local. O(1) check <5ms NFR.
Findings
1. Token-bucket algorithm:
A bucket has capacity (max tokens) and refillRate (tokens/sec). Each request consumes 1 token. On each check: (a) compute elapsed time since last refill, (b) add elapsed * refillRate tokens (capped at capacity), (c) if tokens >= 1, consume 1 and allow; else reject with 429.
M2 parameters:
- User bucket: capacity = 60, refillRate = 1/sec (i.e., 1 token per second, max 60). Note: the spec says "capacity = rate (60 user / 300 tenant)" and "refill 1/sec (user) / 5/sec (tenant)." So user: capacity=60, refill=1/sec (regenerates 60/min). Tenant: capacity=300, refill=5/sec (regenerates 300/min). Both must pass (AND logic): a request is allowed only if BOTH the user bucket and tenant bucket have >= 1 token.
2. TypeScript implementation pattern:
// packages/mcp/rate-limiter.ts (sketch)
interface Bucket { tokens: number; lastRefill: number; }
const userBuckets = new Map<string, Bucket>();
const tenantBuckets = new Map<string, Bucket>();
const USER_CAPACITY = 60, USER_REFILL = 1; // per sec
const TENANT_CAPACITY = 300, TENANT_REFILL = 5;
function checkAndConsume(userId: string, tenantId: string): { allowed: boolean; retryAfterSec?: number } {
const now = Date.now();
const userOk = consume(userBuckets, userId, USER_CAPACITY, USER_REFILL, now);
if (!userOk.allowed) return { allowed: false, retryAfterSec: userOk.retryAfterSec };
const tenantOk = consume(tenantBuckets, tenantId, TENANT_CAPACITY, TENANT_REFILL, now);
if (!tenantOk.allowed) {
// refund the user token since the tenant check failed (fairness)
userBuckets.get(userId)!.tokens += 1;
return { allowed: false, retryAfterSec: tenantOk.retryAfterSec };
}
return { allowed: true };
}
function consume(map: Map<string, Bucket>, key: string, cap: number, refill: number, now: number) {
let b = map.get(key);
if (!b) { b = { tokens: cap, lastRefill: now }; map.set(key, b); }
const elapsedSec = (now - b.lastRefill) / 1000;
b.tokens = Math.min(cap, b.tokens + elapsedSec * refill);
b.lastRefill = now;
if (b.tokens >= 1) { b.tokens -= 1; return { allowed: true }; }
const needed = 1 - b.tokens;
return { allowed: false, retryAfterSec: Math.ceil(needed / refill) };
}
Pitfall: the refund-on-tenant-fail keeps the user bucket from draining when the tenant is the bottleneck. Without it, a tenant at capacity would burn user tokens on every rejected call.
3. RateLimiter interface for M3 Redis swap:
export interface RateLimiter {
checkAndConsume(userId: string, tenantId: string): Promise<{ allowed: boolean; retryAfterSec?: number }>;
}
M2 implements InMemoryRateLimiter (synchronous, wrap in Promise for interface compat). M3 swaps in RedisRateLimiter (sliding window via Redis INCR + EXPIRE, or a Redis-backed token bucket). Config-injectable: the broker takes RateLimiter as a constructor dep. Pitfall: make the interface Promise-returning now even though M2 is sync, so M3 needs no signature change.
4. O(1) check performance (<5ms NFR): The above is O(1) — two Map lookups + arithmetic. Well under 5ms. Pitfall: Map grows unbounded as users/tenants accumulate; add a periodic sweep (e.g., every 5 min, delete buckets idle > 10 min) to bound memory. Not a correctness issue, just hygiene.
5. HTTP 429 + Retry-After header:
On reject, the broker returns HTTP 429 with Retry-After: <seconds> header (integer seconds, per RFC 7231). The body is { error: "rate_limited", retryAfterSec: <n> }. No adapter call is made (REQ-019). The rate-limit check happens BEFORE adapter resolution and BEFORE the write-method blocklist (rate limiting is the outermost gate after auth). Order: auth → tenant resolve → RBAC → rate-limit check → write-method blocklist → adapter resolve → invoke. Pitfall: rate-limit check must come before the audit append for the capability invocation (the 429 is not a capability_invoked event — it's a rate limit rejection; audit it as a separate lightweight event or not at all — the spec doesn't require auditing 429s, and auditing every 429 could amplify a flood. M2 decision: do NOT audit rate-limit rejections (they're not adapter events); the rate limiter logs at warn level).
References
- Token bucket: https://en.wikipedia.org/wiki/Token_bucket
- RFC 7231 Retry-After: https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3
- M1 pattern:
apps/control-plane/app/api/byom/route.ts(auth → action order)
Confidence: 0.90 (well-understood algorithm; the Redis swap interface is the only forward-looking design point).
R-008 — packages/llm-mock for CI LLM smoke (M2 gate item 8)
Scope: M2 gate item 8 (P0): a chat-completion request with tools parameter invokes github.list_repos via the broker, receives adapter response, returns a synthesized LLM response grounded in adapter data. Uses CI-only mock provider (packages/llm-mock).
Findings
1. OpenAI-compatible /v1/chat/completions mock:
The mock is an HTTP server (or a Node handler) that implements POST /v1/chat/completions accepting the OpenAI Chat Completions request shape including the tools parameter (array of {type:"function", function:{name, description, parameters}}). It returns a Chat Completions response. Key: it must mirror the BYOM contract (D-001: OpenAI-compatible /v1/chat/completions). The M1 BYOM validator (packages/byom/validator.ts) already validates this shape — reuse the request/response types from packages/byom/types.ts.
2. The 7-step smoke flow (M2 gate item 8):
- CI sets up the broker with a real GitHub adapter (real PAT, test-org-scoped) + the mock LLM as the BYOM endpoint.
- Smoke test sends
POST /v1/chat/completionsto the mock LLM withtools=[github.list_repos definition]and a prompt like "List my GitHub repositories." - Mock LLM returns
choices[0].message.tool_calls=[{id, type:"function", function:{name:"github.list_repos", arguments:"{}"}}](a tool call, not a final answer). - Broker's translator (
packages/mcp/translator.ts) converts the tool_call to MCPtools/call→ broker routes to the GitHub adapter → real GitHub API call (GET /user/repos) → real repo data. - Broker's translator converts the MCP result back to an OpenAI tool message
{role:"tool", tool_call_id, content:"<repo json>"}. - Smoke test sends a second
POST /v1/chat/completionswithmessages=[original prompt, assistant tool_call, tool message]. - Mock LLM synthesizes a grounded response (e.g., "Your repos are: coreci-chat, coreci-relay...") — the smoke asserts the response text contains real repo names from the GitHub API response.
3. Import-guarding against prod bundles:
packages/llm-mockis adevDependencyofapps/control-plane(or only of the CI test package), NOT adependency.package.jsondevDependenciesaren't installed in prod (pnpm install --prod).- Eslint rule:
no-restricted-importsbanning@coreci/llm-mockinapps/control-plane/app/**andpackages/mcp/**(prod code paths). Allowed only intests/**andpackages/llm-mock/**. - Build-time check: a CI step that greps the prod build output (
dist/or.next/) forllm-mockand fails if found. - Pitfall: the mock must not be imported transitively by a prod dependency. Keep it out of
packages/mcp's dependencies entirely; the broker talks to it over HTTP (as a BYOM endpoint), not via import.
4. How the mock decides which tool to call: The mock is a deterministic test tool, not a real LLM. It pattern-matches the prompt:
- If the prompt contains "list" + "repo" → return
tool_calls:[{function:{name:"github.list_repos", arguments:"{}"}}]. - If the prompt contains "recent" + "run" → return
tool_calls:[{function:{name:"github.get_recent_ci_runs", arguments:'{"owner":"...","repo":"..."}'}}]. - On the second call (with a tool message present), synthesize: "Your repos are: " + parse the repo names from the tool message content + join.
This is hardcoded for the smoke test — not a general LLM. Pitfall: keep the mock simple and deterministic; the smoke test asserts specific repo names appear, so the mock must reliably call github.list_repos on the first turn and synthesize on the second. No randomness.
Implementation pattern
// packages/llm-mock/server.ts (sketch)
async function handleChatCompletion(req, res) {
const { messages, tools } = req.body;
const lastMessage = messages[messages.length - 1];
if (lastMessage.role === "tool") {
// Second turn: synthesize from tool result
const repos = JSON.parse(lastMessage.content);
const names = repos.map(r => r.name).join(", ");
return res.json({ choices: [{ message: { role:"assistant", content:`Your repos are: ${names}` }, finish_reason:"stop" }] });
}
// First turn: emit a tool call
if (tools?.some(t => t.function.name === "github.list_repos")) {
return res.json({ choices: [{ message: { role:"assistant", tool_calls:[{id:"call_1", type:"function", function:{name:"github.list_repos", arguments:"{}"}}] }, finish_reason:"tool_calls" }] });
}
// Fallback
return res.json({ choices: [{ message: { role:"assistant", content:"I don't have a tool for that." }, finish_reason:"stop" }] });
}
Pitfalls
- devDependency only — never a prod dependency.
- Eslint
no-restricted-importsto enforce. - Build-time grep of prod output as backstop.
- Deterministic — no randomness; the smoke must be reproducible.
- Real GitHub target — the smoke hits real GitHub (gate item 7), so it needs a real PAT in CI (Wave 0 prerequisite).
References
- M1 BYOM types:
packages/byom/src/types.ts - M1 BYOM validator pattern:
packages/byom/src/validator.ts - OpenAI Chat Completions API: https://platform.openai.com/docs/api-reference/chat
Confidence: 0.85 (deterministic mock; the 7-step flow is clear; the import-guarding is the main design point).
R-009 — Postgres 16 CI container + RLS verification (Wave 0 prerequisite)
Scope: Wave 0 — CI Postgres 16 container with RLS verification (replaces PGlite-only verification); retroactively validates M1's RLS claims.
Findings (grounded in M1 source: packages/db/)
1. Postgres 16 in CI:
Use Gitea Actions service container (the repo's forge is Gitea at git.cloudinit.dev; Gitea Actions is GitHub Actions-compatible — same YAML, same secrets.*, same service container syntax; or docker-compose for local). Standard pattern:
# .gitea/workflows/test.yml
services:
postgres:
image: postgres:16
env:
POSTGRES_PASSWORD: test
POSTGRES_DB: coreci_test
ports: ["5432:5432"]
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
env:
DATABASE_URL: postgresql://postgres:test@localhost:5432/coreci_test
DB_MODE: pg
The M1 createDb (packages/db/src/create-db.ts) already supports mode: "pg" via the pg Pool — it just needs DATABASE_URL. Pitfall: the pg package must be a dependency (it is, lazily imported). The CI job runs DB_MODE=pg pnpm test so createDb picks the pg path. Pitfall: PGlite tests and pg tests should coexist — run PGlite tests by default (no DATABASE_URL) and pg tests in a separate CI job matrix entry with DB_MODE=pg.
2. RLS verification — does M1's pen-test run against PGlite or Postgres?
Verified from packages/db/tests/pen/cross-tenant.test.ts: it runs against PGlite (createDb({ mode: "pglite" })). The test file explicitly documents: "PGlite 0.5.7 does not enforce RLS policies on SELECT (known limitation of the WASM Postgres build)." The test verifies APPLICATION-LAYER isolation (withTenant + explicit WHERE), not RLS enforcement. The "T1 cannot INSERT a target row for T2" test is a placeholder (expect(true).toBe(true)) with a comment "prod RLS WITH CHECK test runs at M1 review" — that M1 review test never ran against real Postgres in M1 (M1 shipped PGlite-only). Wave 0 must deliver this.
3. How to make the pen-test run against both PGlite and Postgres:
Parameterize the pen-test's createDb call by an env var:
const mode = (process.env.DB_MODE ?? "pglite") as "pglite" | "pg";
const db = await createDb({ mode, databaseUrl: process.env.DATABASE_URL });
Then add PGlite-specific it.skip guards for the RLS-enforcement assertions (the WITH CHECK test) that only run against pg mode:
const isPg = process.env.DB_MODE === "pg";
(it.skip || it)("T1 cannot INSERT a target row for T2 (RLS WITH CHECK)", async () => {
// this only passes under real Postgres with FORCE RLS
await expect(withTenant(T1, c => c.query("INSERT INTO targets (tenant_id, ...) VALUES ($2, ...)", [T2]))).rejects.toThrow();
});
Actually use it with a conditional: run the RLS test only when isPg, else it.skip. The application-layer tests run in both modes. Pitfall: the FORCE ROW LEVEL SECURITY on the app role (migration 0001 lines 132-136) is what makes RLS apply even to the table owner; in PGlite there's no role model so FORCE has no effect. Under real Postgres, the app role (coreci_app) is NOT the owner, so RLS applies automatically; FORCE is belt-and-suspenders.
4. withTenant against real Postgres 16 — does SET LOCAL app.tenant_id work the same?
Yes — this is standard Postgres. SELECT set_config('app.tenant_id', $1, true) (the true = is_local, meaning it lasts only for the current transaction) is the PGlite-compatible way M1 already does it (packages/db/src/withTenant.ts). This works identically in real Postgres 16. Pitfall: M1 uses SELECT set_config(...) not SET LOCAL directly — this is correct and portable. No change needed for pg mode.
5. FORCE RLS on the app role for prod backstop:
The M1 migration 0001 already does ALTER TABLE ... FORCE ROW LEVEL SECURITY for all tenant-scoped tables. For Wave 0 / M2, verify that:
- The
coreci_approle (the runtime role) does NOT haveBYPASSRLS(only themigratorrole does). - A connection as
coreci_appwithapp.tenant_idset to T1 cannot SELECT/INSERT T2's rows (the RLS WITH CHECK test). - The audit_log's
REVOKE UPDATE, DELETEis enforced (the app role cannotUPDATE audit_log).
Pitfall: in CI with postgres:16 and the default postgres superuser, RLS is bypassed (superusers bypass RLS). The CI test must create a non-superuser coreci_app role and connect as it. The migration runner (migrate.ts) connects as migrator (BYPASSRLS) to create tables; the tests connect as coreci_app. This role setup is the main Wave 0 deliverable — a packages/db/scripts/setup-ci-roles.sql that creates coreci_app (no BYPASSRLS) and migrator (BYPASSRLS), run before the migration in CI.
Implementation pattern
packages/db/scripts/setup-ci-roles.sql— createscoreci_appandmigratorroles.packages/db/src/migrate.ts— connects asmigrator(envDATABASE_URL_MIGRATOR).packages/db/tests/pen/cross-tenant.test.ts— parameterized byDB_MODE; RLS-enforcement tests gated onisPg.- CI workflow — two jobs:
test-pglite(default) andtest-postgres(service container +DB_MODE=pg+ role setup).
Pitfalls
- Superuser bypasses RLS — CI must use a non-superuser role for the app connection.
set_config(..., true)is portable (PGlite + pg).- M1's pen-test has placeholder assertions — replace with real RLS WITH CHECK assertions gated on pg mode.
- Migration role vs app role — migrator has BYPASSRLS, app does not.
References
- Postgres RLS: https://www.postgresql.org/docs/16/ddl-rowsecurity.html
- GitHub Actions service containers: https://docs.github.com/en/actions/using-containerized-services/creating-postgresql-service-containers
- M1 source:
packages/db/src/withTenant.ts,packages/db/src/create-db.ts,packages/db/migrations/0001_init.sql,packages/db/tests/pen/cross-tenant.test.ts
Confidence: 0.90 (standard Postgres CI pattern; M1 code is portable; the role setup is the only new artifact).
Conformance verification artifact (R-001 summary)
Per M2 gate item 15, the M2 gate must produce recorded evidence that the broker conforms to MCP 2025-06-18. The artifact consists of:
tests/mcp-conformance/test suite (6 tests, all must pass):tools-list.test.ts—GET /api/mcp/toolsreturns 9 tools with{name, description, inputSchema}matching REQ-015 exactly.tools-call-happy.test.ts— mock adapter invocation returns MCP result shape{content:[{type:"text",text}], isError:false}via SSE.tools-call-error.test.ts— mock adapterisError:truereturns MCP error shape via SSEerrorterminal event.tools-call-invalid-args.test.ts— args failinginputSchema→ HTTP 400 schema-validation error (broker rejects before adapter).translator.test.ts— OpenAItool_calls↔ MCPtools/callbidirectional translation, includingargumentsstring→object parse andisError→content prefix.lifecycle.test.ts— in-process custom transport syntheticinitialize/initializedhandshake preserves JSON-RPC 2.0 envelope.
packages/mcp/PROTOCOL.mddocumenting:- Pinned spec version
2025-06-18with links to the three spec pages (tools, transports, lifecycle). - Transports used: in-process custom (broker↔adapters), stdio (broker↔CI LLM smoke), REST facade + SSE (broker↔UI — NOT Streamable HTTP, compliant as custom transport).
- JSON-RPC 2.0 shapes preserved:
tools/list,tools/callrequest/response envelopes. - OpenAI ↔ MCP translation contract (the typed
translator.tsmodule). - The synthetic lifecycle handshake for in-process adapters (recommendation: implement for conformance evidence).
- Pinned spec version
MCP_PROTOCOL_VERSION = "2025-06-18"constant exported frompackages/mcpand asserted in the conformance test header.
This satisfies gate item 15. The artifact is test evidence + documentation, not a third-party conformance suite (MCP has no official conformance test suite as of 2025-06-18; the modelcontextprotocol.io spec is the authoritative reference, verified verbatim in R-001).
Closing summary
Confidence per area
| Area | Confidence | Notes |
|---|---|---|
| R-001 MCP conformance | 0.80 | Spec verified verbatim; synthetic lifecycle handshake is a recommendation, not a mandate. Lowest-confidence area resolved with documented artifact. |
| R-002 Proxmox VE API | 0.80 | API verified; PVEAuditor introspection is a PVE gap — pragmatic validation documented. |
| R-003 SSH via Relay Agent | 0.85 | Grounded in M1 source; tool_call message is a clean protocol extension. |
| R-004 GitHub REST API | 0.78 | API verified; fine-grained PAT scope introspection is a GitHub gap — best-effort + per-invocation 403 handling documented. |
| R-005 Gitea REST API | 0.72 | Gitea docs page required JS (didn't fetch); findings from spec + known Gitea conventions. Recommend verifying against a running Gitea 1.22+ and <1.22 instance during Wave I. |
| R-006 SSE Next.js | 0.85 | Standard pattern; M1 route handler pattern confirmed. |
| R-007 Token-bucket | 0.90 | Well-understood algorithm; Redis swap interface designed. |
| R-008 llm-mock | 0.85 | Deterministic mock; 7-step flow clear; import-guarding designed. |
| R-009 Postgres 16 CI | 0.90 | Standard CI pattern; M1 code is portable; role setup is the only new artifact. |
Flagged risks (escalate to PLAN/GRILL)
-
R-001 (MCP conformance) — RESOLVED but document: the in-process custom transport needs a synthetic
initialize/initializedhandshake to produce clean conformance evidence. This is an implementation recommendation, not a spec violation if omitted — but omitting it leaves the lowest-confidence gate item (15) weaker. Action: implement the synthetic handshake in Wave F. -
R-002 (PVEAuditor validation) — PVE gap: Proxmox has no clean "what role does this token have" introspection endpoint. The M2 broker validates "token works for reads" (
GET /version+GET /nodes), not "token lacks writes." The broker's write-method blocklist (reject POST/PUT/DELETE) is the load-bearing safety boundary. Action: document this in the adapter config UI help text; the operator is responsible for creating a PVEAuditor-scoped token. Confidence 0.70 on this sub-point. -
R-004 (GitHub fine-grained PAT scopes) — GitHub gap: GitHub has no public API to introspect a fine-grained PAT's granted scopes. The M2 broker validates token validity (
GET /user→ implicitmetadata:read) at submit, and handlesactions:readper-invocation via 403 +X-Accepted-GitHub-Permissionsheader. Classic PATs are rejected by prefix (github_pat_required). Action: document in UI; per-tool 403 handling in the adapter. Confidence 0.75 on scope validation. -
R-005 (Gitea) — needs runtime verification: the Gitea docs page did not fetch cleanly (JS-required). Findings are from the M2 spec + known Gitea API conventions (which mirror GitHub). Action: during Wave I, verify
GET /api/v1/version,GET /api/v1/user/repos,GET /api/v1/repos/{owner}/{repo}/actions/runsagainst a running Gitea 1.22+ and a <1.22 instance. Confirm theread:repositoryscope behavior and theAuthorization: token <token>header. -
R-003 (SSH defense-in-depth) — two independent implementations: the broker (TypeScript) and Relay Agent (Go) each implement whitelist validation independently. Action: keep them semantically in sync (6-command subset is the broker layer; M1's broader whitelist is the agent layer) but code-independent. Add a cross-layer test in the M2 gate that asserts a non-whitelisted command is rejected by BOTH layers.
-
R-006 (SSE) — stream-not-opened orphan risk: if
POST /api/mcp/invokeis called but the client never opensGET /api/mcp/stream/:correlationId, the adapter call runs but events buffer. Action: 30s stream-not-opened timeout in the stream manager to cancel orphan adapter calls. -
R-009 (Postgres CI) — role setup required: the M1 pen-test has placeholder RLS assertions (
expect(true).toBe(true)) because PGlite doesn't enforce RLS on SELECT. Action: Wave 0 must deliver thecoreci_app/migratorrole setup and replace the placeholder with real RLS WITH CHECK assertions gated onDB_MODE=pg.
Decisions logged (to DecisionEngine)
- D-M2-R001: MCP
2025-06-18in-process custom transport is spec-compliant IF JSON-RPC 2.0 shape + synthetic lifecycle handshake are preserved. REST facade + SSE for broker↔UI is a compliant custom transport (NOT Streamable HTTP). Confidence 0.80. - D-M2-R002: PVEAuditor validation at submit = "token works for reads" (
GET /version+GET /nodes), NOT "token lacks writes." Broker write-method blocklist is the load-bearing boundary. Confidence 0.70. - D-M2-R003: SSH broker-side whitelist = independent 6-command TypeScript validation; Relay Agent
CheckCommand= M1's broader Go whitelist. Both must pass. No shell in Go executor. Confidence 0.85. - D-M2-R004: GitHub fine-grained PAT scope validation =
github_pat_prefix check +GET /userat submit (implicit metadata:read) + per-invocation 403/X-Accepted-GitHub-Permissionshandling for actions:read. Classic PATs rejected. Confidence 0.75. - D-M2-R005: Gitea version-aware:
GET /api/v1/version→ ≥1.22 requiresread:repository(validate viaGET /api/v1/user/repos?limit=1); <1.22 accepts any token + broker-side write-method blocklist. Confidence 0.72 (verify at runtime in Wave I). - D-M2-R006: SSE per-call streams, ULID correlation IDs, 30s stream-not-opened timeout, no audit on client cancel. Confidence 0.85.
- D-M2-R007: Token-bucket in-memory, Promise-returning
RateLimiterinterface for M3 Redis swap, refund-on-tenant-fail for fairness, no audit on 429. Confidence 0.90. - D-M2-R008:
packages/llm-mockas devDependency, eslintno-restricted-imports, deterministic pattern-matching mock, 7-step smoke against real GitHub. Confidence 0.85. - D-M2-R009: CI Postgres 16 service container +
coreci_app/migratorrole setup, parameterized pen-test (DB_MODE), real RLS WITH CHECK assertions gated on pg mode. Confidence 0.90.
All above the 0.6 threshold. No escalations required. Pipeline proceeds to PLAN.
End of M2 Research Findings. M1 research preserved in git history (commit prior to M2 overwrite).