Files
CIAgent 0c15d3d0b2 docs(milestone): complete M2 — MCP Layer & Day 1 Adapters (v0.2)
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---
2026-08-25 06:14:21 +00:00

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 identifier
  • title (optional, string) — NEW in 2025-06-18 — human-readable display name (not present in older spec drafts). The broker SHOULD populate title for the Test-Call UI but it is not required for conformance.
  • description (optional, string) — human-readable description
  • inputSchema (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 resultcontent[] array of content items (text/image/audio/resource_link/resource) + isError: false + optional structuredContent (JSON object, when outputSchema provided).

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:

  1. JSON-RPC 2.0 message formattools/list and tools/call requests/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).
  2. Lifecycle requirements — initialization (capability negotiation + version agreement), operation, shutdown. The M2 broker↔adapter in-process transport must implement a synthetic initialize/initialized handshake on adapter registration, OR document that single-process adapters skip lifecycle because they share the broker process. Recommendation: implement a lightweight synthetic initialize exchange 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 MCP tools/call-result-shaped ({content:[{type:"text",text}], isError}) inside the REST/SSE envelope. The POST /api/mcp/invoke{correlationId, streamUrl}GET /api/mcp/stream/:correlationId flow returns SSE events whose data field 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 } } where arguments is a JSON string.
  • MCP: { method:"tools/call", params: { name, arguments } } where arguments is a parsed JSON object.
  • Translation: tool_calls[i].function.nameparams.name; JSON.parse(tool_calls[i].function.arguments)params.arguments. Pitfall: OpenAI sends arguments as a string; MCP expects an object. The translator MUST JSON.parse and 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 } (or isError: true).
  • OpenAI: a follow-up messages[] entry { role:"tool", tool_call_id, content } where content is a string. If the LLM should treat it as an error, OpenAI has no native isError — the convention is to put the error text in content and 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 passes isError through 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:

  1. tools-list.test.ts — asserts GET /api/mcp/tools returns an array where each tool has {name, description, inputSchema} (JSON Schema object with type:"object"), and the 9-tool closed set matches REQ-015 exactly. Snapshot the full tools/list response.
  2. tools-call-happy.test.ts — invokes a mock adapter via POST /api/mcp/invoke, asserts the SSE stream emits a tool_result event whose data parses to {content:[{type:"text",text}], isError:false} — the MCP result shape.
  3. tools-call-error.test.ts — invokes a mock adapter that returns isError:true, asserts the SSE error terminal event carries the MCP error shape.
  4. tools-call-invalid-args.test.ts — invokes a tool with args not matching inputSchema, 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).
  5. translator.test.ts — asserts the OpenAI↔MCP translator: tool_calls[].function.{name, arguments(JSON string)}params.{name, arguments(object)} and result.content[].text + isError → OpenAI {role:"tool", tool_call_id, content}.
  6. lifecycle.test.ts — asserts the in-process custom transport performs the synthetic initialize/initialized handshake on adapter registration and preserves JSON-RPC 2.0 envelope shape.
  7. Spec-version pin: a constant MCP_PROTOCOL_VERSION = "2025-06-18" exported from packages/mcp and asserted in the conformance test header. A comment linking to https://modelcontextprotocol.io/specification/2025-06-18/server/tools and .../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

  • title and outputSchema are new in 2025-06-18 — older references show the schema without them. Pin to 2025-06-18 and document the version in code.
  • arguments type mismatch — OpenAI string vs MCP object. The translator MUST parse.
  • isError is 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-Id or MCP-Protocol-Version HTTP headers on the REST facade (those are Streamable HTTP transport specifics).
  • Paginationtools/list supports cursor. M2's closed 9-tool set is small enough to return in one page (no nextCursor); the broker should omit nextCursor when there are no more pages.

References

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/ticket with username=root@pam&password=... → returns {data:{ticket, CSRFPreventionToken, username}}. The ticket is set as cookie PVEAuthCookie=<ticket>. Write requests (POST/PUT/DELETE) require the CSRFPreventionToken header. 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 Authorization header to PVEAPIToken=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 to PVEAuditor role, the broker stores the full PVEAPIToken=... string via SecretProvider, and the adapter sends it as the Authorization header 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: false for PVE specifically (configurable per-adapter; default true in prod, but a allowSelfSigned config flag for PVE since it's the common case). Security note: this is per-adapter config, stored in the mcp_adapters.config JSON column, NOT a global setting. The broker validates it's only set for Proxmox adapters.
  • No cookie handling needed for token auth — just the Authorization header on every request.
  • Use the global fetch (Node 18+) with AbortSignal.timeout(10_000) for the 10s upstream NFR (mirrors packages/byom/validator.ts pattern).
  • Response shape: { data: <payload> } — the adapter unwraps data. Errors: PVE returns { data: null, errors: "..." } with HTTP 5xx, or HTTP 200 with {data: null} for some not-found cases. Pitfall: check both !res.ok AND body.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

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-plane ws-server.ts handles upgrade).
  • Auth: Authorization: Bearer <tenantToken> on handshake (JWT relay registration token, verified via verifyRelayToken).
  • 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 handleMessage switch has a default case that returns an error for unknown types. M2 adds a tool_call message type — the control-plane ws-server.ts must add a tool_call handler, and the Go agent's reader goroutine must route tool_call messages to a new execution path.

2. M1 CheckCommand whitelist hook (verified from apps/relay-agent/whitelist/whitelist.go):

  • Signature: CheckCommand(cmd string) errorTHIS IS THE LOCKED G-004 CONTRACT. M2's SSH adapter calls this BEFORE constructing exec.Command. Any change requires a documented migration.
  • The whitelist JSON (ssh-whitelist.json) is versioned (version: 1) with commands (allowed command prefixes) and arguments.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's CheckCommand validates 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> — prefix systemctl status followed 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 like systemctl status nginx; rm -rf /.
  • journalctl -n <N>journalctl -n followed 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's CheckCommand rejection 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 shellexec.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_calltool_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.Command with 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_call message type is additive; M1's register/ping/pong must 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 show 2026-03-10 as the latest, but 2022-11-28 is the stable GA version; M2 should use 2022-11-28 for stability).
  • Response 200: array of Minimal Repository objects — 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). The Link header contains next/prev URLs. M2 decision: github.list_repos (inventory, 60s cache) fetches per_page=100 and follows Link next 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_repos takes 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] }. Each WorkflowRun: 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). status is one of completed|in_progress|queued|...; conclusion is one of success|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: owner and repo are 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 WorkflowRun object (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):

  1. Detect classic vs fine-grained: Classic PATs start with ghp_ (or gho_/ghu_); fine-grained PATs start with github_pat_. The broker rejects classic PATs at submit (D-006: fine-grained only — classic repo scope grants write). Pitfall: the token prefix is the discriminator. If the token doesn't start with github_pat_, reject with HTTP 422 "fine-grained PAT required."
  2. Validate the token works + has metadata:read: call GET /user with the token. If 401 → invalid token (HTTP 422). If 200 → token is valid. All fine-grained PATs require metadata:read implicitly (it's mandatory on every fine-grained PAT), so a successful GET /user implies metadata:read.
  3. Validate actions:read: call GET /user/repos?per_page=1 — wait, this requires metadata:read (which we have). To validate actions:read specifically, attempt GET /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 validates actions:read at invocation time per-tool: when github.get_recent_ci_runs or github.get_workflow_run is called, if GitHub returns 403 with X-Accepted-GitHub-Permissions indicating actions=read was required, the broker surfaces HTTP 403 + adapter.write_rejected audit event... NO — a 403 for missing actions:read is a scope-mismatch, not a write attempt. Refinement: the broker distinguishes: (a) 403 from GitHub for missing scope → HTTP 403 "insufficient scope" + audit adapter.capability_invoked with result=failure (NOT write_rejected — no write was attempted); (b) the broker's own write-method blocklist (rejecting POST/PUT/DELETE) → adapter.write_rejected. These are different.
  4. Submit-time best effort: call GET /user (validates token + implicit metadata:read). Record validated=true. The actions:read is validated on first get_recent_ci_runs/get_workflow_run invocation. The UI help text says "ensure the PAT has actions: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 after x-ratelimit-reset.
  • Secondary rate limits: 100 concurrent, 900 points/min (GET=1pt, POST=5pt). Exceeding → 403/429 with retry-after header.
  • M2 adapter behavior: observe x-ratelimit-remaining; if it hits 0, do NOT make the call — return HTTP 429 to the caller with Retry-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 rejectiongithub_pat_ prefix check at submit.
  • X-GitHub-Api-Version — pin to 2022-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

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 version and compares to 1.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 the mcp_adapters.config JSON column at submit time.

2. GET /api/v1/user/repos?limit=50 — list repos:

  • Auth: Authorization: token <token> (Gitea uses token not Bearer). 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). M2 gitea.list_repos returns 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=true in 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> not Bearer (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 allowSelfSigned per-adapter flag as Proxmox.
  • No gitea.get_workflow_run in M2 (deferred to v1.2+ per Q2) — only list_repos + get_recent_ci_runs.

References

  • Gitea API Swagger: https://gitea.com/api/swagger (and /api/swagger on any Gitea instance)
  • Gitea API is auto-documented; the OpenAPI spec is at /swagger.v1.json on 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 ulid npm package (ulid() returns a 26-char Crockford-base32 string, lexicographically sortable by time). Add as a dependency to packages/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), use monotonicFactory().
  • The correlation ID format: 01HXXXXXXXXXXXXXXXXXXXXXX (26 chars). The SSE id appends -<seq>: 01HXXXXXXXXXXXXXXXXXXXXXX-0, ...-1, etc.

5. Correlation context management:

  • In-memory Map<correlationId, CorrelationContext> in packages/mcp/stream-manager.ts. Each context holds: { correlationId, tenantId, userId, adapterType, toolName, controller?: ReadableStreamController, abortController: AbortController, createdAt }.
  • POST /api/mcp/invoke mints 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/:correlationId looks up the context, attaches the req'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/invoke returns 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: if GET /api/mcp/stream/:correlationId isn't called within 30s of POST /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-dynamic to 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

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

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):

  1. CI sets up the broker with a real GitHub adapter (real PAT, test-org-scoped) + the mock LLM as the BYOM endpoint.
  2. Smoke test sends POST /v1/chat/completions to the mock LLM with tools=[github.list_repos definition] and a prompt like "List my GitHub repositories."
  3. Mock LLM returns choices[0].message.tool_calls=[{id, type:"function", function:{name:"github.list_repos", arguments:"{}"}}] (a tool call, not a final answer).
  4. Broker's translator (packages/mcp/translator.ts) converts the tool_call to MCP tools/call → broker routes to the GitHub adapter → real GitHub API call (GET /user/repos) → real repo data.
  5. Broker's translator converts the MCP result back to an OpenAI tool message {role:"tool", tool_call_id, content:"<repo json>"}.
  6. Smoke test sends a second POST /v1/chat/completions with messages=[original prompt, assistant tool_call, tool message].
  7. 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-mock is a devDependency of apps/control-plane (or only of the CI test package), NOT a dependency. package.json devDependencies aren't installed in prod (pnpm install --prod).
  • Eslint rule: no-restricted-imports banning @coreci/llm-mock in apps/control-plane/app/** and packages/mcp/** (prod code paths). Allowed only in tests/** and packages/llm-mock/**.
  • Build-time check: a CI step that greps the prod build output (dist/ or .next/) for llm-mock and 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-imports to 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

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_app role (the runtime role) does NOT have BYPASSRLS (only the migrator role does).
  • A connection as coreci_app with app.tenant_id set to T1 cannot SELECT/INSERT T2's rows (the RLS WITH CHECK test).
  • The audit_log's REVOKE UPDATE, DELETE is enforced (the app role cannot UPDATE 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 — creates coreci_app and migrator roles.
  • packages/db/src/migrate.ts — connects as migrator (env DATABASE_URL_MIGRATOR).
  • packages/db/tests/pen/cross-tenant.test.ts — parameterized by DB_MODE; RLS-enforcement tests gated on isPg.
  • CI workflow — two jobs: test-pglite (default) and test-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

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:

  1. tests/mcp-conformance/ test suite (6 tests, all must pass):
    • tools-list.test.tsGET /api/mcp/tools returns 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 adapter isError:true returns MCP error shape via SSE error terminal event.
    • tools-call-invalid-args.test.ts — args failing inputSchema → HTTP 400 schema-validation error (broker rejects before adapter).
    • translator.test.ts — OpenAI tool_calls ↔ MCP tools/call bidirectional translation, including arguments string→object parse and isError→content prefix.
    • lifecycle.test.ts — in-process custom transport synthetic initialize/initialized handshake preserves JSON-RPC 2.0 envelope.
  2. packages/mcp/PROTOCOL.md documenting:
    • Pinned spec version 2025-06-18 with 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/call request/response envelopes.
    • OpenAI ↔ MCP translation contract (the typed translator.ts module).
    • The synthetic lifecycle handshake for in-process adapters (recommendation: implement for conformance evidence).
  3. MCP_PROTOCOL_VERSION = "2025-06-18" constant exported from packages/mcp and 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)

  1. R-001 (MCP conformance) — RESOLVED but document: the in-process custom transport needs a synthetic initialize/initialized handshake 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.

  2. 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.

  3. 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 → implicit metadata:read) at submit, and handles actions:read per-invocation via 403 + X-Accepted-GitHub-Permissions header. 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.

  4. 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/runs against a running Gitea 1.22+ and a <1.22 instance. Confirm the read:repository scope behavior and the Authorization: token <token> header.

  5. 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.

  6. R-006 (SSE) — stream-not-opened orphan risk: if POST /api/mcp/invoke is called but the client never opens GET /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.

  7. 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 the coreci_app / migrator role setup and replace the placeholder with real RLS WITH CHECK assertions gated on DB_MODE=pg.

Decisions logged (to DecisionEngine)

  • D-M2-R001: MCP 2025-06-18 in-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 /user at submit (implicit metadata:read) + per-invocation 403/X-Accepted-GitHub-Permissions handling for actions:read. Classic PATs rejected. Confidence 0.75.
  • D-M2-R005: Gitea version-aware: GET /api/v1/version → ≥1.22 requires read:repository (validate via GET /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 RateLimiter interface for M3 Redis swap, refund-on-tenant-fail for fairness, no audit on 429. Confidence 0.90.
  • D-M2-R008: packages/llm-mock as devDependency, eslint no-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/migrator role 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).