mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 06:40:44 +00:00
fix(acpx): avoid per-session MCP on openclaw bridge
This commit is contained in:
alexlomt
committed by
Peter Steinberger
Peter Steinberger
parent
f79c6ab607
commit
0bcd390546
@@ -7,19 +7,32 @@ type TestSessionStore = {
|
||||
save(record: Record<string, unknown>): Promise<void>;
|
||||
};
|
||||
|
||||
function makeRuntime(baseStore: TestSessionStore): {
|
||||
function makeRuntime(
|
||||
baseStore: TestSessionStore,
|
||||
options: Partial<ConstructorParameters<typeof AcpxRuntime>[0]> = {},
|
||||
): {
|
||||
runtime: AcpxRuntime;
|
||||
wrappedStore: TestSessionStore & { markFresh: (sessionKey: string) => void };
|
||||
delegate: { close: AcpRuntime["close"] };
|
||||
delegate: {
|
||||
close: AcpRuntime["close"];
|
||||
ensureSession: AcpRuntime["ensureSession"];
|
||||
getStatus: NonNullable<AcpRuntime["getStatus"]>;
|
||||
};
|
||||
bridgeSafeDelegate: {
|
||||
close: AcpRuntime["close"];
|
||||
ensureSession: AcpRuntime["ensureSession"];
|
||||
getStatus: NonNullable<AcpRuntime["getStatus"]>;
|
||||
};
|
||||
} {
|
||||
const runtime = new AcpxRuntime({
|
||||
cwd: "/tmp",
|
||||
sessionStore: baseStore,
|
||||
agentRegistry: {
|
||||
resolve: () => "codex",
|
||||
list: () => ["codex"],
|
||||
resolve: (agentName: string) => (agentName === "openclaw" ? "openclaw acp" : agentName),
|
||||
list: () => ["codex", "openclaw"],
|
||||
},
|
||||
permissionMode: "approve-reads",
|
||||
...options,
|
||||
});
|
||||
|
||||
return {
|
||||
@@ -29,7 +42,24 @@ function makeRuntime(baseStore: TestSessionStore): {
|
||||
sessionStore: TestSessionStore & { markFresh: (sessionKey: string) => void };
|
||||
}
|
||||
).sessionStore,
|
||||
delegate: (runtime as unknown as { delegate: { close: AcpRuntime["close"] } }).delegate,
|
||||
delegate: (
|
||||
runtime as unknown as {
|
||||
delegate: {
|
||||
close: AcpRuntime["close"];
|
||||
ensureSession: AcpRuntime["ensureSession"];
|
||||
getStatus: NonNullable<AcpRuntime["getStatus"]>;
|
||||
};
|
||||
}
|
||||
).delegate,
|
||||
bridgeSafeDelegate: (
|
||||
runtime as unknown as {
|
||||
bridgeSafeDelegate: {
|
||||
close: AcpRuntime["close"];
|
||||
ensureSession: AcpRuntime["ensureSession"];
|
||||
getStatus: NonNullable<AcpRuntime["getStatus"]>;
|
||||
};
|
||||
}
|
||||
).bridgeSafeDelegate,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -102,4 +132,164 @@ describe("AcpxRuntime fresh reset wrapper", () => {
|
||||
expect(await wrappedStore.load("agent:codex:acp:binding:test")).toBeUndefined();
|
||||
expect(baseStore.load).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("routes openclaw ensureSession through the bridge-safe delegate when MCP servers are configured", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => undefined),
|
||||
save: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
const { runtime, delegate, bridgeSafeDelegate } = makeRuntime(baseStore, {
|
||||
mcpServers: [{ name: "tools", command: "mcp-tools" }] as never,
|
||||
});
|
||||
const defaultEnsure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "default",
|
||||
});
|
||||
const bridgeEnsure = vi.spyOn(bridgeSafeDelegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "bridge",
|
||||
});
|
||||
|
||||
const result = await runtime.ensureSession({
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
agent: "openclaw",
|
||||
mode: "persistent",
|
||||
});
|
||||
|
||||
expect(result.runtimeSessionName).toBe("bridge");
|
||||
expect(bridgeEnsure).toHaveBeenCalledOnce();
|
||||
expect(defaultEnsure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("routes non-openclaw sessions through the default delegate", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => undefined),
|
||||
save: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
const { runtime, delegate, bridgeSafeDelegate } = makeRuntime(baseStore, {
|
||||
mcpServers: [{ name: "tools", command: "mcp-tools" }] as never,
|
||||
});
|
||||
const defaultEnsure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "default",
|
||||
});
|
||||
const bridgeEnsure = vi.spyOn(bridgeSafeDelegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "bridge",
|
||||
});
|
||||
|
||||
const result = await runtime.ensureSession({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
});
|
||||
|
||||
expect(result.runtimeSessionName).toBe("default");
|
||||
expect(defaultEnsure).toHaveBeenCalledOnce();
|
||||
expect(bridgeEnsure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("routes handle-based follow-up calls for openclaw sessions through the bridge-safe delegate", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => undefined),
|
||||
save: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
const { runtime, delegate, bridgeSafeDelegate } = makeRuntime(baseStore, {
|
||||
mcpServers: [{ name: "tools", command: "mcp-tools" }] as never,
|
||||
});
|
||||
const defaultStatus = vi.spyOn(delegate, "getStatus").mockResolvedValue({
|
||||
summary: "default",
|
||||
});
|
||||
const bridgeStatus = vi.spyOn(bridgeSafeDelegate, "getStatus").mockResolvedValue({
|
||||
summary: "bridge",
|
||||
});
|
||||
const handle: Parameters<NonNullable<AcpRuntime["getStatus"]>>[0]["handle"] = {
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "openclaw-session-handle",
|
||||
};
|
||||
|
||||
const status = await runtime.getStatus({ handle });
|
||||
|
||||
expect(status.summary).toBe("bridge");
|
||||
expect(bridgeStatus).toHaveBeenCalledWith({ handle });
|
||||
expect(defaultStatus).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps MCP-enabled routing when the openclaw agent is overridden to a non-bridge adapter", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => undefined),
|
||||
save: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
const { runtime, delegate, bridgeSafeDelegate } = makeRuntime(baseStore, {
|
||||
mcpServers: [{ name: "tools", command: "mcp-tools" }] as never,
|
||||
agentRegistry: {
|
||||
resolve: (agentName: string) => (agentName === "openclaw" ? "codex" : agentName),
|
||||
list: () => ["codex", "openclaw"],
|
||||
},
|
||||
});
|
||||
const defaultEnsure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "default",
|
||||
});
|
||||
const bridgeEnsure = vi.spyOn(bridgeSafeDelegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "bridge",
|
||||
});
|
||||
|
||||
const result = await runtime.ensureSession({
|
||||
sessionKey: "agent:openclaw:acp:test",
|
||||
agent: "openclaw",
|
||||
mode: "persistent",
|
||||
});
|
||||
|
||||
expect(result.runtimeSessionName).toBe("default");
|
||||
expect(defaultEnsure).toHaveBeenCalledOnce();
|
||||
expect(bridgeEnsure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses the bridge-safe delegate for any agent mapped to the openclaw bridge command", async () => {
|
||||
const baseStore: TestSessionStore = {
|
||||
load: vi.fn(async () => undefined),
|
||||
save: vi.fn(async () => {}),
|
||||
};
|
||||
|
||||
const { runtime, delegate, bridgeSafeDelegate } = makeRuntime(baseStore, {
|
||||
mcpServers: [{ name: "tools", command: "mcp-tools" }] as never,
|
||||
agentRegistry: {
|
||||
resolve: (agentName: string) => (agentName === "codex" ? "openclaw acp" : agentName),
|
||||
list: () => ["codex", "openclaw"],
|
||||
},
|
||||
});
|
||||
const defaultEnsure = vi.spyOn(delegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "default",
|
||||
});
|
||||
const bridgeEnsure = vi.spyOn(bridgeSafeDelegate, "ensureSession").mockResolvedValue({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
backend: "acpx",
|
||||
runtimeSessionName: "bridge",
|
||||
});
|
||||
|
||||
const result = await runtime.ensureSession({
|
||||
sessionKey: "agent:codex:acp:test",
|
||||
agent: "codex",
|
||||
mode: "persistent",
|
||||
});
|
||||
|
||||
expect(result.runtimeSessionName).toBe("bridge");
|
||||
expect(bridgeEnsure).toHaveBeenCalledOnce();
|
||||
expect(defaultEnsure).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,28 +58,96 @@ function createResetAwareSessionStore(baseStore: AcpSessionStore): ResetAwareSes
|
||||
};
|
||||
}
|
||||
|
||||
type AcpxRuntimeLike = AcpRuntime & {
|
||||
probeAvailability(): Promise<void>;
|
||||
isHealthy(): boolean;
|
||||
doctor(): Promise<AcpRuntimeDoctorReport>;
|
||||
};
|
||||
const OPENCLAW_BRIDGE_COMMAND = "openclaw acp";
|
||||
|
||||
export class AcpxRuntime implements AcpxRuntimeLike {
|
||||
function normalizeAgentName(value: string | undefined): string | undefined {
|
||||
const normalized = value?.trim().toLowerCase();
|
||||
return normalized ? normalized : undefined;
|
||||
}
|
||||
|
||||
function readAgentFromSessionKey(sessionKey: string | undefined): string | undefined {
|
||||
const normalized = sessionKey?.trim();
|
||||
if (!normalized) {
|
||||
return undefined;
|
||||
}
|
||||
const match = /^agent:(?<agent>[^:]+):/i.exec(normalized);
|
||||
return normalizeAgentName(match?.groups?.agent);
|
||||
}
|
||||
|
||||
function readAgentFromHandle(handle: AcpRuntimeHandle): string | undefined {
|
||||
const decoded = decodeAcpxRuntimeHandleState(handle.runtimeSessionName);
|
||||
if (typeof decoded === "object" && decoded !== null) {
|
||||
const { agent } = decoded as { agent?: unknown };
|
||||
if (typeof agent === "string") {
|
||||
return normalizeAgentName(agent) ?? readAgentFromSessionKey(handle.sessionKey);
|
||||
}
|
||||
}
|
||||
return readAgentFromSessionKey(handle.sessionKey);
|
||||
}
|
||||
|
||||
function resolveAgentCommand(params: {
|
||||
agentName: string | undefined;
|
||||
agentRegistry: AcpAgentRegistry;
|
||||
}): string | undefined {
|
||||
const normalizedAgentName = normalizeAgentName(params.agentName);
|
||||
if (!normalizedAgentName) {
|
||||
return undefined;
|
||||
}
|
||||
const resolvedCommand = params.agentRegistry.resolve(normalizedAgentName);
|
||||
return typeof resolvedCommand === "string" ? resolvedCommand.trim() || undefined : undefined;
|
||||
}
|
||||
|
||||
function shouldUseBridgeSafeDelegate(params: {
|
||||
agentName: string | undefined;
|
||||
agentRegistry: AcpAgentRegistry;
|
||||
}): boolean {
|
||||
return resolveAgentCommand(params) === OPENCLAW_BRIDGE_COMMAND;
|
||||
}
|
||||
|
||||
function shouldUseDistinctBridgeDelegate(options: AcpRuntimeOptions): boolean {
|
||||
const { mcpServers } = options as { mcpServers?: unknown };
|
||||
return Array.isArray(mcpServers) && mcpServers.length > 0;
|
||||
}
|
||||
|
||||
export class AcpxRuntime implements AcpRuntime {
|
||||
private readonly sessionStore: ResetAwareSessionStore;
|
||||
private readonly agentRegistry: AcpAgentRegistry;
|
||||
private readonly delegate: BaseAcpxRuntime;
|
||||
private readonly bridgeSafeDelegate: BaseAcpxRuntime;
|
||||
|
||||
constructor(
|
||||
options: AcpRuntimeOptions,
|
||||
testOptions?: ConstructorParameters<typeof BaseAcpxRuntime>[1],
|
||||
) {
|
||||
this.sessionStore = createResetAwareSessionStore(options.sessionStore);
|
||||
this.delegate = new BaseAcpxRuntime(
|
||||
{
|
||||
...options,
|
||||
sessionStore: this.sessionStore,
|
||||
},
|
||||
testOptions,
|
||||
);
|
||||
this.agentRegistry = options.agentRegistry;
|
||||
const sharedOptions = {
|
||||
...options,
|
||||
sessionStore: this.sessionStore,
|
||||
};
|
||||
this.delegate = new BaseAcpxRuntime(sharedOptions, testOptions);
|
||||
this.bridgeSafeDelegate = shouldUseDistinctBridgeDelegate(options)
|
||||
? new BaseAcpxRuntime(
|
||||
{
|
||||
...sharedOptions,
|
||||
mcpServers: [],
|
||||
},
|
||||
testOptions,
|
||||
)
|
||||
: this.delegate;
|
||||
}
|
||||
|
||||
private resolveDelegateForAgent(agentName: string | undefined): BaseAcpxRuntime {
|
||||
return shouldUseBridgeSafeDelegate({
|
||||
agentName,
|
||||
agentRegistry: this.agentRegistry,
|
||||
})
|
||||
? this.bridgeSafeDelegate
|
||||
: this.delegate;
|
||||
}
|
||||
|
||||
private resolveDelegateForHandle(handle: AcpRuntimeHandle): BaseAcpxRuntime {
|
||||
return this.resolveDelegateForAgent(readAgentFromHandle(handle));
|
||||
}
|
||||
|
||||
isHealthy(): boolean {
|
||||
@@ -95,11 +163,11 @@ export class AcpxRuntime implements AcpxRuntimeLike {
|
||||
}
|
||||
|
||||
ensureSession(input: Parameters<AcpRuntime["ensureSession"]>[0]): Promise<AcpRuntimeHandle> {
|
||||
return this.delegate.ensureSession(input);
|
||||
return this.resolveDelegateForAgent(input.agent).ensureSession(input);
|
||||
}
|
||||
|
||||
runTurn(input: Parameters<AcpRuntime["runTurn"]>[0]): AsyncIterable<AcpRuntimeEvent> {
|
||||
return this.delegate.runTurn(input);
|
||||
return this.resolveDelegateForHandle(input.handle).runTurn(input);
|
||||
}
|
||||
|
||||
getCapabilities(): ReturnType<BaseAcpxRuntime["getCapabilities"]> {
|
||||
@@ -107,19 +175,19 @@ export class AcpxRuntime implements AcpxRuntimeLike {
|
||||
}
|
||||
|
||||
getStatus(input: Parameters<NonNullable<AcpRuntime["getStatus"]>>[0]): Promise<AcpRuntimeStatus> {
|
||||
return this.delegate.getStatus(input);
|
||||
return this.resolveDelegateForHandle(input.handle).getStatus(input);
|
||||
}
|
||||
|
||||
setMode(input: Parameters<NonNullable<AcpRuntime["setMode"]>>[0]): Promise<void> {
|
||||
return this.delegate.setMode(input);
|
||||
return this.resolveDelegateForHandle(input.handle).setMode(input);
|
||||
}
|
||||
|
||||
setConfigOption(input: Parameters<NonNullable<AcpRuntime["setConfigOption"]>>[0]): Promise<void> {
|
||||
return this.delegate.setConfigOption(input);
|
||||
return this.resolveDelegateForHandle(input.handle).setConfigOption(input);
|
||||
}
|
||||
|
||||
cancel(input: Parameters<AcpRuntime["cancel"]>[0]): Promise<void> {
|
||||
return this.delegate.cancel(input);
|
||||
return this.resolveDelegateForHandle(input.handle).cancel(input);
|
||||
}
|
||||
|
||||
async prepareFreshSession(input: { sessionKey: string }): Promise<void> {
|
||||
@@ -127,7 +195,7 @@ export class AcpxRuntime implements AcpxRuntimeLike {
|
||||
}
|
||||
|
||||
close(input: Parameters<AcpRuntime["close"]>[0]): Promise<void> {
|
||||
return this.delegate
|
||||
return this.resolveDelegateForHandle(input.handle)
|
||||
.close({
|
||||
handle: input.handle,
|
||||
reason: input.reason,
|
||||
|
||||
Reference in New Issue
Block a user