mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
fix(mcp): treat an empty or unfilled seat as no seat
An app that doesn't fill in its plugin settings passes the seat on as written.
VS Code and Codex do this: OPENRIG_SESSION_NAME arrives as
"${user_config.seat}". rig mcp serve then acted as a seat with that name,
stamping it as the sender of every write. An empty seat fell back to
RIGGED_SESSION_NAME, so the server could act as a seat inherited from the
app's parent environment.
At startup, rig mcp serve now treats an empty value, or one still holding a
placeholder (${...} or a leading $NAME), as no seat. It removes both names
from its environment, so every reader sees no seat: request stamping,
rig_whoami and the HTTP startup line. No address can contain a placeholder.
A set OPENRIG_SESSION_NAME decides, and the legacy name counts only when
it's absent. Real seat values are unchanged.
The server says so on stderr. rig_whoami answers seat: null and names the
ignored value, says writes that need an author are refused (actor_required),
and says how to set the seat.
This commit is contained in:
1 parent
338c97b5d7
commit
7409ee21a1
5 files changed
+90
-11
No files matched your search
@@ -2,7 +2,7 @@ import { Command } from "commander";
|
||||
import { serveStdio } from "@modelcontextprotocol/server/stdio";
|
||||
import { DaemonClient } from "../client.js";
|
||||
import { getDaemonStatus, getDaemonUrl, type LifecycleDeps , daemonStatusGuard} from "../daemon-lifecycle.js";
|
||||
import { createMcpServer } from "../mcp-server.js";
|
||||
import { createMcpServer, dropUnfilledSeat, unfilledSeatText, type UnfilledSeat } from "../mcp-server.js";
|
||||
import { addGrant, McpHttpConfigError, mcpTokenFilePath, readGrantServiceHeader, readGrants, revokeGrant, startMcpHttpServer } from "../mcp-http.js";
|
||||
import { readOpenRigEnv } from "../openrig-compat.js";
|
||||
import { realDeps } from "./daemon.js";
|
||||
@@ -33,6 +33,9 @@ export function mcpCommand(depsOverride?: StatusDeps): Command {
|
||||
Both MCP protocol revisions are served, over stdio and --http alike: clients on 2026-07-28 (which may start with
|
||||
server/discover) and clients that open with 2025's initialize handshake.
|
||||
|
||||
An empty OPENRIG_SESSION_NAME, or one an app passed on unfilled (such as \${user_config.seat}), means no seat: the
|
||||
server acts as none, never as a seat named after the placeholder or one inherited as RIGGED_SESSION_NAME.
|
||||
|
||||
With --http and no token file, the server stays on loopback and acts as OPENRIG_SESSION_NAME, declared and not
|
||||
authenticated, like stdio. Once a grant exists (rig mcp token add), every request needs its bearer credential, loopback
|
||||
included, and each caller acts as its grant's seat within its rigs and roots. Add a grant before reaching the port
|
||||
@@ -60,6 +63,8 @@ be https with no userinfo, query or fragment, written as a URL writes them (a lo
|
||||
stays the endpoint; a proxy in front forwards the metadata path unchanged.`)
|
||||
.action(async (opts: { port?: string; http?: boolean; bind?: string; grantService?: string; grantServiceHeaderFile?: string; taskDestination?: string; oauthResource?: string; oauthIssuer?: string; frontDoor?: boolean }) => {
|
||||
const deps = getDepsF();
|
||||
const unfilledSeat = dropUnfilledSeat();
|
||||
if (unfilledSeat) console.error(`rig mcp serve: ${unfilledSeatText(unfilledSeat)}; serving with no seat until it names this app's address on the team.`);
|
||||
|
||||
let daemonUrl: string;
|
||||
if (opts.port) {
|
||||
@@ -106,7 +111,7 @@ stays the endpoint; a proxy in front forwards the metadata path unchanged.`)
|
||||
return;
|
||||
}
|
||||
if (opts.http) {
|
||||
await serveHttp(daemonUrl, opts.bind ?? "127.0.0.1:7437", opts.grantService, opts.grantServiceHeaderFile, oauth, opts.taskDestination, opts.frontDoor === true);
|
||||
await serveHttp(daemonUrl, opts.bind ?? "127.0.0.1:7437", opts.grantService, opts.grantServiceHeaderFile, oauth, opts.taskDestination, opts.frontDoor === true, unfilledSeat);
|
||||
return;
|
||||
}
|
||||
const client = deps.clientFactory(daemonUrl);
|
||||
@@ -116,7 +121,7 @@ stays the endpoint; a proxy in front forwards the metadata path unchanged.`)
|
||||
process.once("SIGINT", finish);
|
||||
process.once("SIGTERM", finish);
|
||||
// Serves the protocol revision the client opens with: 2026-07-28 per request, or 2025's initialize handshake.
|
||||
const served = serveStdio(() => createMcpServer(client, { frontDoor: opts.frontDoor === true }), { onerror: (err) => console.error(`rig mcp serve: ${err.message}`) });
|
||||
const served = serveStdio(() => createMcpServer(client, { frontDoor: opts.frontDoor === true, ...(unfilledSeat ? { unfilledSeat } : {}) }), { onerror: (err) => console.error(`rig mcp serve: ${err.message}`) });
|
||||
try {
|
||||
// EOF is a normal stdio disconnect, not an unresolved top-level await.
|
||||
if (process.stdin.readableEnded) finish();
|
||||
@@ -183,7 +188,7 @@ stays the endpoint; a proxy in front forwards the metadata path unchanged.`)
|
||||
}
|
||||
|
||||
async function serveHttp(daemonUrl: string, bind: string, grantService?: string, grantServiceHeaderFile?: string,
|
||||
oauth?: { resource: string; issuer: string }, taskDestination?: string, frontDoor = false): Promise<void> {
|
||||
oauth?: { resource: string; issuer: string }, taskDestination?: string, frontDoor = false, unfilledSeat?: UnfilledSeat): Promise<void> {
|
||||
const match = /^(\[[^\]]+\]|[^:]+):(\d+)$/.exec(bind);
|
||||
if (!match) {
|
||||
console.error(`--bind takes host:port (for example 127.0.0.1:7437 or [::1]:7437), not ${bind}`);
|
||||
@@ -196,6 +201,7 @@ async function serveHttp(daemonUrl: string, bind: string, grantService?: string,
|
||||
try {
|
||||
const header = grantServiceHeaderFile ? readGrantServiceHeader(grantServiceHeaderFile) : undefined;
|
||||
served = await startMcpHttpServer({ daemonUrl, host, port: Number(match[2]), tokenFile, taskDestination, frontDoor,
|
||||
...(unfilledSeat ? { unfilledSeat } : {}),
|
||||
...(grantService ? { grantService: { url: grantService, ...(header ? { header } : {}) } } : {}), ...(oauth ? { oauth } : {}) });
|
||||
} catch (err) {
|
||||
console.error(err instanceof McpHttpConfigError ? err.message : `Can't serve on ${bind}: ${(err as Error).message}`);
|
||||
|
||||
@@ -19,7 +19,7 @@ import { createHash, randomBytes, timingSafeEqual } from "node:crypto";
|
||||
import { createMcpHandler, type AuthInfo } from "@modelcontextprotocol/server";
|
||||
import { toNodeHandler } from "@modelcontextprotocol/node";
|
||||
import { DaemonClient } from "./client.js";
|
||||
import { createMcpServer, type McpAccess } from "./mcp-server.js";
|
||||
import { createMcpServer, type McpAccess, type UnfilledSeat } from "./mcp-server.js";
|
||||
import { getOpenRigHome } from "./openrig-compat.js";
|
||||
import { MCP_PRESENCE_FILE, presenceRecorder } from "./mcp-presence.js";
|
||||
import { resolveRigHandle } from "./rig-handle.js";
|
||||
@@ -314,6 +314,8 @@ export type McpHttpOptions = {
|
||||
taskDestination?: string;
|
||||
/** rig mcp serve --front-door: a caller that isn't external gets the front-door instructions, not the member workflow. */
|
||||
frontDoor?: boolean;
|
||||
/** The seat value rig mcp serve dropped as empty or unfilled, which rig_whoami names to a caller with no seat. */
|
||||
unfilledSeat?: UnfilledSeat;
|
||||
/** With grantService: OAuth discovery for clients that sign in with OAuth. The public resource this server is reached
|
||||
* as and the authorization server that issues its keys, both served exactly as given. */
|
||||
oauth?: { resource: string; issuer: string };
|
||||
@@ -393,7 +395,8 @@ export async function startMcpHttpServer(opts: McpHttpOptions): Promise<{ url: s
|
||||
// Never a default seat: a request reaches the handler only with its caller decided.
|
||||
if (!caller) throw new Error("a request reached the MCP handler without its caller");
|
||||
return createMcpServer(makeClient(opts.daemonUrl, caller.seat, caller.principal), { seat: caller.seat, access: caller.access,
|
||||
taskDestination: opts.taskDestination, presenceFile: () => presenceFile, frontDoor: opts.frontDoor === true });
|
||||
taskDestination: opts.taskDestination, presenceFile: () => presenceFile, frontDoor: opts.frontDoor === true,
|
||||
...(opts.unfilledSeat ? { unfilledSeat: opts.unfilledSeat } : {}) });
|
||||
});
|
||||
const serveMcp = toNodeHandler(mcpHandler);
|
||||
const bearer = (req: http.IncomingMessage): string | undefined => /^Bearer\s+(\S+)\s*$/i.exec(req.headers.authorization ?? "")?.[1];
|
||||
|
||||
@@ -662,12 +662,37 @@ function mapResult(
|
||||
return opts.structured ? { content: [{ type: "text", text }], structuredContent: structured(res.data) } : { content: [{ type: "text", text }] };
|
||||
}
|
||||
|
||||
/** A seat value an app passed on without filling it in: empty, or still a placeholder. */
|
||||
export interface UnfilledSeat { variable: "OPENRIG_SESSION_NAME" | "RIGGED_SESSION_NAME"; value: string }
|
||||
|
||||
// An app that doesn't substitute its settings passes the template through as written: ${user_config.seat}, ${input:seat},
|
||||
// $SEAT. No seat address can contain these.
|
||||
const SEAT_PLACEHOLDER = /\$\{[^}]*\}|^\s*\$[A-Za-z_]/;
|
||||
|
||||
/** Treat an empty or unfilled seat as no seat, for the whole process: drops both names from env, so neither requests
|
||||
* nor rig_whoami act as the placeholder, or as a seat inherited under the legacy name behind an empty one. A set
|
||||
* OPENRIG_SESSION_NAME decides; RIGGED_SESSION_NAME counts only when it's absent. Returns what was dropped. */
|
||||
export function dropUnfilledSeat(env: NodeJS.ProcessEnv = process.env): UnfilledSeat | undefined {
|
||||
const variable = env.OPENRIG_SESSION_NAME !== undefined ? "OPENRIG_SESSION_NAME" : env.RIGGED_SESSION_NAME !== undefined ? "RIGGED_SESSION_NAME" : undefined;
|
||||
const value = variable ? env[variable]! : undefined;
|
||||
if (!variable || value === undefined || (value.trim() && !SEAT_PLACEHOLDER.test(value))) return undefined;
|
||||
delete env.OPENRIG_SESSION_NAME;
|
||||
delete env.RIGGED_SESSION_NAME;
|
||||
return { variable, value };
|
||||
}
|
||||
|
||||
export function unfilledSeatText(seat: UnfilledSeat): string {
|
||||
return seat.value.trim()
|
||||
? `${seat.variable} is ${JSON.stringify(seat.value)}, a placeholder the app didn't fill in`
|
||||
: `${seat.variable} is empty`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates an MCP server wrapping the daemon HTTP API.
|
||||
* @param client - DaemonClient connected to the daemon
|
||||
* @returns McpServer instance (not yet connected to a transport)
|
||||
*/
|
||||
export function createMcpServer(client: DaemonClient, options: { seat?: string; access?: McpAccess; taskDestination?: string; rostersFolder?: () => string; presenceFile?: () => string; frontDoor?: boolean } = {}): McpServer {
|
||||
export function createMcpServer(client: DaemonClient, options: { seat?: string; access?: McpAccess; taskDestination?: string; rostersFolder?: () => string; presenceFile?: () => string; frontDoor?: boolean; unfilledSeat?: UnfilledSeat } = {}): McpServer {
|
||||
const server = new McpServer({
|
||||
name: "openrig",
|
||||
version: CLI_VERSION,
|
||||
@@ -1537,7 +1562,9 @@ export function createMcpServer(client: DaemonClient, options: { seat?: string;
|
||||
try {
|
||||
const seat = seatOf();
|
||||
if (!seat) {
|
||||
const none = { ok: true, seat: null, caller, message: "This server has no seat: OPENRIG_SESSION_NAME isn't set where it was configured, so its writes aren't attributed to a seat." };
|
||||
const none = options.unfilledSeat
|
||||
? { ok: true, seat: null, caller, unfilledSeat: options.unfilledSeat, message: `This server has no seat: ${unfilledSeatText(options.unfilledSeat)}, so it acts as no seat. Reads work; the team refuses writes that need an author (actor_required). Set OPENRIG_SESSION_NAME where the server is configured to this app's address on the team, such as cursor@my-team.` }
|
||||
: { ok: true, seat: null, caller, message: "This server has no seat: OPENRIG_SESSION_NAME isn't set where it was configured, so its writes aren't attributed to a seat." };
|
||||
return { content: [{ type: "text" as const, text: JSON.stringify(none) }], structuredContent: none };
|
||||
}
|
||||
const params = new URLSearchParams({ sessionName: seat, compact: "1" });
|
||||
|
||||
@@ -4,8 +4,8 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
|
||||
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
|
||||
import { createMcpServer, FRONT_DOOR_WORKFLOW, FRONT_DOOR_WORKFLOW_VERSION, MEMBER_WORKFLOW, MEMBER_WORKFLOW_VERSION, type McpAccess } from "../src/mcp-server.js";
|
||||
import { DaemonClient, DaemonConnectionError, type DaemonResponse } from "../src/client.js";
|
||||
import { createMcpServer, dropUnfilledSeat, FRONT_DOOR_WORKFLOW, FRONT_DOOR_WORKFLOW_VERSION, MEMBER_WORKFLOW, MEMBER_WORKFLOW_VERSION, type McpAccess, type UnfilledSeat } from "../src/mcp-server.js";
|
||||
import { DaemonClient, DaemonConnectionError, senderIdentityHeaders, type DaemonResponse } from "../src/client.js";
|
||||
|
||||
function mockClient(overrides?: {
|
||||
get?: (path: string) => Promise<DaemonResponse>;
|
||||
@@ -366,6 +366,47 @@ describe("MCP Server", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("an empty or unfilled seat, as an app that doesn't fill in its settings passes it, is no seat: never the placeholder or an inherited seat", async () => {
|
||||
// What an app hands the server when it doesn't substitute its plugin settings, and what stays a seat.
|
||||
const cases: Array<[NodeJS.ProcessEnv, UnfilledSeat | undefined]> = [
|
||||
[{ OPENRIG_SESSION_NAME: "${user_config.seat}" }, { variable: "OPENRIG_SESSION_NAME", value: "${user_config.seat}" }],
|
||||
[{ OPENRIG_SESSION_NAME: "${input:seat}" }, { variable: "OPENRIG_SESSION_NAME", value: "${input:seat}" }],
|
||||
[{ OPENRIG_SESSION_NAME: "$SEAT" }, { variable: "OPENRIG_SESSION_NAME", value: "$SEAT" }],
|
||||
// An empty seat doesn't fall back to one inherited under the legacy name.
|
||||
[{ OPENRIG_SESSION_NAME: "", RIGGED_SESSION_NAME: "dev-impl@my-rig" }, { variable: "OPENRIG_SESSION_NAME", value: "" }],
|
||||
[{ RIGGED_SESSION_NAME: "${user_config.seat}" }, { variable: "RIGGED_SESSION_NAME", value: "${user_config.seat}" }],
|
||||
[{ OPENRIG_SESSION_NAME: "claude-app@my-rig" }, undefined],
|
||||
[{ RIGGED_SESSION_NAME: "claude-app@my-rig" }, undefined],
|
||||
[{}, undefined],
|
||||
];
|
||||
for (const [given, dropped] of cases) {
|
||||
const env = { ...given };
|
||||
expect(dropUnfilledSeat(env), JSON.stringify(given)).toEqual(dropped);
|
||||
expect(env, JSON.stringify(given)).toEqual(dropped ? {} : given);
|
||||
}
|
||||
|
||||
try {
|
||||
vi.stubEnv("OPENRIG_SESSION_NAME", "${user_config.seat}");
|
||||
vi.stubEnv("RIGGED_SESSION_NAME", "");
|
||||
const unfilledSeat = dropUnfilledSeat()!;
|
||||
// No request carries the placeholder as its sender, and whoami asks no daemon about it.
|
||||
expect(senderIdentityHeaders()).toEqual({});
|
||||
const quietGet = vi.fn(async () => ({ status: 200, data: {} }));
|
||||
await setup({ get: quietGet }, { frontDoor: true, unfilledSeat });
|
||||
const whoami = await mcpClient.callTool({ name: "rig_whoami", arguments: {} });
|
||||
expect(whoami.isError).toBeFalsy();
|
||||
expect(whoami.structuredContent).toMatchObject({ ok: true, seat: null, caller: { authenticated: false },
|
||||
unfilledSeat: { variable: "OPENRIG_SESSION_NAME", value: "${user_config.seat}" } });
|
||||
const message = (whoami.structuredContent as { message: string }).message;
|
||||
expect(message).toContain("OPENRIG_SESSION_NAME is \"${user_config.seat}\", a placeholder the app didn't fill in");
|
||||
expect(message).toContain("such as cursor@my-team");
|
||||
expect(quietGet).not.toHaveBeenCalled();
|
||||
await cleanup();
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
}
|
||||
});
|
||||
|
||||
it("the queue tools return structured content; every error a tool returns carries one shape and keeps its old text", async () => {
|
||||
// The queue's missing-item read answers with its code alone, as GET /api/queue/:qitemId does.
|
||||
const missing = { error: "qitem_not_found" };
|
||||
|
||||
@@ -10,7 +10,9 @@ This plugin is for a person's own agent in an app such as Claude Code or Codex.
|
||||
|
||||
- **The MCP server entry**, in the Claude Code manifest only: `rig mcp serve --front-door`, acting as the seat you set in
|
||||
the plugin's settings. Claude Code substitutes that seat into the server's environment. Codex doesn't, so the plugin
|
||||
ships no shared MCP file, and Codex gets the server from one command (below).
|
||||
ships no shared MCP file, and Codex gets the server from one command (below). An app that passes the seat on unfilled
|
||||
(`${user_config.seat}`) or empty gets a server with no seat: `rig_whoami` says why, and the team refuses its writes
|
||||
until a seat is set. It never acts as a seat named after the placeholder.
|
||||
- **`--front-door`** tells your agent the team's questions to its seat are yours to answer: it shows them to you and
|
||||
records only what you say, instead of working them itself as a team member would.
|
||||
- **One skill,** `working-with-an-openrig-team`: the loop around the tools. It covers handing work to the team's lead,
|
||||
|
||||
Reference in new issue
Block a user