Fix OMP slash commands and skills loading (#1698)
* Fix OMP slash command discovery Keep Pi on get_commands while allowing the OMP-backed provider to use get_available_commands. Both providers reject the other command without echoing an id, so the adapter needs an explicit command-list RPC setting instead of a timeout-based fallback. * Address OMP command test review
This commit is contained in:
@@ -26,6 +26,8 @@ Pi import discovery reads Pi's persisted JSONL session files because Pi RPC does
|
|||||||
|
|
||||||
OMP is a built-in Pi-compatible provider, disabled by default. It uses the `omp` command and imports terminal-started sessions from `~/.omp/agent/sessions` when enabled. Other Pi-compatible forks can still be custom providers that extend `pi`, override `command`, and set `params.sessionDir` to their JSONL session directory.
|
OMP is a built-in Pi-compatible provider, disabled by default. It uses the `omp` command and imports terminal-started sessions from `~/.omp/agent/sessions` when enabled. Other Pi-compatible forks can still be custom providers that extend `pi`, override `command`, and set `params.sessionDir` to their JSONL session directory.
|
||||||
|
|
||||||
|
Pi and OMP currently use different RPC names for slash-command discovery. The Pi package accepts `get_commands`; OMP accepts `get_available_commands`. Keep this as an explicit adapter setting for the built-in provider instead of probing with a fallback, because both packages return unknown-command errors without the request `id`, which otherwise turns a fast mismatch into the normal RPC timeout.
|
||||||
|
|
||||||
Pi RPC extension UI dialog requests (`select`, `input`, `editor`, `confirm`) are bridged into Paseo question permissions and answered with `extension_ui_response`. Pi extensions such as `ask_user` may chain dialogs: for example, a `select` can be followed by an optional-comment `input`. When an `ask_user` tool call declares `allowComment: true`, Paseo presents the selection and optional comment as one question permission, answers Pi's initial `select` immediately, then auto-answers the follow-up optional `input` with the comment the user already supplied (or an empty string). Preserve placeholders and optional/skip semantics for standalone optional inputs so the app can still distinguish "skip this optional input" from "cancel the whole dialog." Fire-and-forget extension UI requests such as notifications are intentionally ignored by the provider adapter unless Paseo grows first-class UI for them.
|
Pi RPC extension UI dialog requests (`select`, `input`, `editor`, `confirm`) are bridged into Paseo question permissions and answered with `extension_ui_response`. Pi extensions such as `ask_user` may chain dialogs: for example, a `select` can be followed by an optional-comment `input`. When an `ask_user` tool call declares `allowComment: true`, Paseo presents the selection and optional comment as one question permission, answers Pi's initial `select` immediately, then auto-answers the follow-up optional `input` with the comment the user already supplied (or an empty string). Preserve placeholders and optional/skip semantics for standalone optional inputs so the app can still distinguish "skip this optional input" from "cancel the whole dialog." Fire-and-forget extension UI requests such as notifications are intentionally ignored by the provider adapter unless Paseo grows first-class UI for them.
|
||||||
|
|
||||||
OpenCode MCP injection is dynamic and session-scoped. Call OpenCode's `mcp.add` endpoint with the MCP server config and do not follow it with `mcp.connect`; `connect` only toggles MCP servers already present in OpenCode's own config. New OpenCode versions return `McpServerNotFoundError`/404 for `connect` after a dynamic add because the server is not config-backed, while older versions silently swallowed the same missing-config path.
|
OpenCode MCP injection is dynamic and session-scoped. Call OpenCode's `mcp.add` endpoint with the MCP server config and do not follow it with `mcp.connect`; `connect` only toggles MCP servers already present in OpenCode's own config. New OpenCode versions return `McpServerNotFoundError`/404 for `connect` after a dynamic add because the server is not config-backed, while older versions silently swallowed the same missing-config path.
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ const mockState = vi.hoisted(() => {
|
|||||||
interface ConstructorEntry {
|
interface ConstructorEntry {
|
||||||
runtimeSettings?: unknown;
|
runtimeSettings?: unknown;
|
||||||
providerParams?: unknown;
|
providerParams?: unknown;
|
||||||
|
commandsRpcType?: unknown;
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@@ -214,12 +215,20 @@ vi.mock("./providers/pi/agent.js", () => ({
|
|||||||
readonly provider = "pi";
|
readonly provider = "pi";
|
||||||
readonly runtimeSettings?: unknown;
|
readonly runtimeSettings?: unknown;
|
||||||
|
|
||||||
constructor(options: { runtimeSettings?: unknown; providerParams?: unknown }) {
|
constructor(options: {
|
||||||
|
runtimeSettings?: unknown;
|
||||||
|
providerParams?: unknown;
|
||||||
|
commandsRpcType?: unknown;
|
||||||
|
}) {
|
||||||
this.runtimeSettings = options.runtimeSettings;
|
this.runtimeSettings = options.runtimeSettings;
|
||||||
mockState.constructorArgs.pi.push({
|
const entry: ConstructorEntry = {
|
||||||
runtimeSettings: options.runtimeSettings,
|
runtimeSettings: options.runtimeSettings,
|
||||||
providerParams: options.providerParams,
|
providerParams: options.providerParams,
|
||||||
});
|
};
|
||||||
|
if (options.commandsRpcType !== undefined) {
|
||||||
|
entry.commandsRpcType = options.commandsRpcType;
|
||||||
|
}
|
||||||
|
mockState.constructorArgs.pi.push(entry);
|
||||||
}
|
}
|
||||||
|
|
||||||
async createSession(): Promise<never> {
|
async createSession(): Promise<never> {
|
||||||
@@ -458,6 +467,7 @@ test("OMP is a disabled built-in backed by the Pi adapter", () => {
|
|||||||
providerParams: {
|
providerParams: {
|
||||||
sessionDir: "~/.omp/agent/sessions",
|
sessionDir: "~/.omp/agent/sessions",
|
||||||
},
|
},
|
||||||
|
commandsRpcType: "get_available_commands",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -488,11 +498,10 @@ test("new provider extending claude appears in registry", () => {
|
|||||||
expect(registry.zai.createClient(logger).provider).toBe("zai");
|
expect(registry.zai.createClient(logger).provider).toBe("zai");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("new provider extending pi passes params to the base provider constructor", () => {
|
test("built-in OMP override passes params to the Pi adapter constructor", () => {
|
||||||
const registry = buildProviderRegistry(logger, {
|
const registry = buildProviderRegistry(logger, {
|
||||||
providerOverrides: {
|
providerOverrides: {
|
||||||
omp: {
|
omp: {
|
||||||
extends: "pi",
|
|
||||||
label: "OMP",
|
label: "OMP",
|
||||||
command: ["omp"],
|
command: ["omp"],
|
||||||
params: {
|
params: {
|
||||||
@@ -515,6 +524,7 @@ test("new provider extending pi passes params to the base provider constructor",
|
|||||||
providerParams: {
|
providerParams: {
|
||||||
sessionDir: "~/.omp/agent/sessions",
|
sessionDir: "~/.omp/agent/sessions",
|
||||||
},
|
},
|
||||||
|
commandsRpcType: "get_available_commands",
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -156,6 +156,7 @@ const PROVIDER_CLIENT_FACTORIES: Record<string, ProviderClientFactory> = {
|
|||||||
providerParams: options?.providerParams ?? {
|
providerParams: options?.providerParams ?? {
|
||||||
sessionDir: "~/.omp/agent/sessions",
|
sessionDir: "~/.omp/agent/sessions",
|
||||||
},
|
},
|
||||||
|
commandsRpcType: "get_available_commands",
|
||||||
}),
|
}),
|
||||||
mock: (logger) => new MockLoadTestAgentClient(logger),
|
mock: (logger) => new MockLoadTestAgentClient(logger),
|
||||||
"mock-slow": () => new MockSlowProviderClient(),
|
"mock-slow": () => new MockSlowProviderClient(),
|
||||||
|
|||||||
@@ -62,6 +62,7 @@ import type { PiRuntime, PiRuntimeSession } from "./runtime.js";
|
|||||||
import type {
|
import type {
|
||||||
PiAgentSessionEvent,
|
PiAgentSessionEvent,
|
||||||
PiAgentMessage,
|
PiAgentMessage,
|
||||||
|
PiCommandsRpcType,
|
||||||
PiImageContent,
|
PiImageContent,
|
||||||
PiModel,
|
PiModel,
|
||||||
PiRpcSlashCommand,
|
PiRpcSlashCommand,
|
||||||
@@ -154,6 +155,7 @@ interface PiRpcAgentClientOptions {
|
|||||||
logger: Logger;
|
logger: Logger;
|
||||||
runtimeSettings?: ProviderRuntimeSettings;
|
runtimeSettings?: ProviderRuntimeSettings;
|
||||||
providerParams?: unknown;
|
providerParams?: unknown;
|
||||||
|
commandsRpcType?: PiCommandsRpcType;
|
||||||
runtime?: PiRuntime;
|
runtime?: PiRuntime;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -967,8 +969,12 @@ function mapPiModel(model: PiModel): AgentModelDefinition {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
function createRuntime(logger: Logger, runtimeSettings?: ProviderRuntimeSettings): PiRuntime {
|
function createRuntime(
|
||||||
return new PiCliRuntime({ logger, runtimeSettings });
|
logger: Logger,
|
||||||
|
runtimeSettings?: ProviderRuntimeSettings,
|
||||||
|
commandsRpcType?: PiCommandsRpcType,
|
||||||
|
): PiRuntime {
|
||||||
|
return new PiCliRuntime({ logger, runtimeSettings, commandsRpcType });
|
||||||
}
|
}
|
||||||
|
|
||||||
export class PiRpcAgentSession implements AgentSession {
|
export class PiRpcAgentSession implements AgentSession {
|
||||||
@@ -1866,7 +1872,9 @@ export class PiRpcAgentClient implements AgentClient {
|
|||||||
this.logger = options.logger;
|
this.logger = options.logger;
|
||||||
this.runtimeSettings = options.runtimeSettings;
|
this.runtimeSettings = options.runtimeSettings;
|
||||||
this.providerParams = PiProviderParamsSchema.parse(options.providerParams ?? {});
|
this.providerParams = PiProviderParamsSchema.parse(options.providerParams ?? {});
|
||||||
this.runtime = options.runtime ?? createRuntime(options.logger, options.runtimeSettings);
|
this.runtime =
|
||||||
|
options.runtime ??
|
||||||
|
createRuntime(options.logger, options.runtimeSettings, options.commandsRpcType);
|
||||||
}
|
}
|
||||||
|
|
||||||
async createSession(
|
async createSession(
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import pino from "pino";
|
|||||||
import { describe, expect, test } from "vitest";
|
import { describe, expect, test } from "vitest";
|
||||||
|
|
||||||
import { PiCliRuntime } from "./cli-runtime.js";
|
import { PiCliRuntime } from "./cli-runtime.js";
|
||||||
|
import type { PiCommandsRpcType } from "./rpc-types.js";
|
||||||
import type { PiRuntimeLaunch } from "./runtime.js";
|
import type { PiRuntimeLaunch } from "./runtime.js";
|
||||||
|
|
||||||
type PiChild = ChildProcessWithoutNullStreams & {
|
type PiChild = ChildProcessWithoutNullStreams & {
|
||||||
@@ -31,10 +32,15 @@ function createPiChild(): PiChild {
|
|||||||
return child;
|
return child;
|
||||||
}
|
}
|
||||||
|
|
||||||
function createRuntime(child: PiChild, launches: PiRuntimeLaunch[] = []): PiCliRuntime {
|
function createRuntime(
|
||||||
|
child: PiChild,
|
||||||
|
launches: PiRuntimeLaunch[] = [],
|
||||||
|
options?: { commandsRpcType?: PiCommandsRpcType },
|
||||||
|
): PiCliRuntime {
|
||||||
return new PiCliRuntime({
|
return new PiCliRuntime({
|
||||||
logger: pino({ level: "silent" }),
|
logger: pino({ level: "silent" }),
|
||||||
command: ["pi"],
|
command: ["pi"],
|
||||||
|
commandsRpcType: options?.commandsRpcType,
|
||||||
spawnProcess: (launch) => {
|
spawnProcess: (launch) => {
|
||||||
launches.push(launch);
|
launches.push(launch);
|
||||||
return child;
|
return child;
|
||||||
@@ -184,6 +190,42 @@ describe("PiCliRuntime", () => {
|
|||||||
expect(events).toEqual([{ type: "turn_start" }]);
|
expect(events).toEqual([{ type: "turn_start" }]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("lists commands through the default Pi get_commands RPC", async () => {
|
||||||
|
const child = createPiChild();
|
||||||
|
const commandTypes: string[] = [];
|
||||||
|
replyToCommands(child, (command) => {
|
||||||
|
commandTypes.push(String(command.type));
|
||||||
|
return {
|
||||||
|
commands: [{ name: "review", description: "Review changes", source: "extension" }],
|
||||||
|
};
|
||||||
|
});
|
||||||
|
const session = await createRuntime(child).startSession({ cwd: "/workspace/project" });
|
||||||
|
|
||||||
|
await expect(session.getCommands()).resolves.toEqual([
|
||||||
|
{ name: "review", description: "Review changes", source: "extension" },
|
||||||
|
]);
|
||||||
|
expect(commandTypes).toEqual(["get_commands"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("lists commands through the OMP-compatible get_available_commands RPC", async () => {
|
||||||
|
const child = createPiChild();
|
||||||
|
const commandTypes: string[] = [];
|
||||||
|
replyToCommands(child, (command) => {
|
||||||
|
commandTypes.push(String(command.type));
|
||||||
|
return {
|
||||||
|
commands: [{ name: "skill:ctx-stats", description: "Show context stats", source: "skill" }],
|
||||||
|
};
|
||||||
|
});
|
||||||
|
const session = await createRuntime(child, [], {
|
||||||
|
commandsRpcType: "get_available_commands",
|
||||||
|
}).startSession({ cwd: "/workspace/project" });
|
||||||
|
|
||||||
|
await expect(session.getCommands()).resolves.toEqual([
|
||||||
|
{ name: "skill:ctx-stats", description: "Show context stats", source: "skill" },
|
||||||
|
]);
|
||||||
|
expect(commandTypes).toEqual(["get_available_commands"]);
|
||||||
|
});
|
||||||
|
|
||||||
test("keeps unicode line separators inside one JSONL record", async () => {
|
test("keeps unicode line separators inside one JSONL record", async () => {
|
||||||
const child = createPiChild();
|
const child = createPiChild();
|
||||||
replyToCommands(child, () => ({}));
|
replyToCommands(child, () => ({}));
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ import {
|
|||||||
} from "./runtime.js";
|
} from "./runtime.js";
|
||||||
import type {
|
import type {
|
||||||
PiAgentMessage,
|
PiAgentMessage,
|
||||||
|
PiCommandsRpcType,
|
||||||
PiModel,
|
PiModel,
|
||||||
PiRpcCommand,
|
PiRpcCommand,
|
||||||
PiRpcResponse,
|
PiRpcResponse,
|
||||||
@@ -26,6 +27,7 @@ const DEFAULT_PI_COMMAND: [string, ...string[]] = [
|
|||||||
process.env.PI_COMMAND ?? process.env.PI_ACP_PI_COMMAND ?? "pi",
|
process.env.PI_COMMAND ?? process.env.PI_ACP_PI_COMMAND ?? "pi",
|
||||||
];
|
];
|
||||||
const DEFAULT_TIMEOUT_MS = 30_000;
|
const DEFAULT_TIMEOUT_MS = 30_000;
|
||||||
|
const DEFAULT_COMMANDS_RPC_TYPE: PiCommandsRpcType = "get_commands";
|
||||||
const STDERR_BUFFER_LIMIT = 8192;
|
const STDERR_BUFFER_LIMIT = 8192;
|
||||||
const GRACEFUL_SHUTDOWN_TIMEOUT_MS = 2_000;
|
const GRACEFUL_SHUTDOWN_TIMEOUT_MS = 2_000;
|
||||||
const FORCE_SHUTDOWN_TIMEOUT_MS = 1_000;
|
const FORCE_SHUTDOWN_TIMEOUT_MS = 1_000;
|
||||||
@@ -48,15 +50,18 @@ export interface PiCliRuntimeOptions {
|
|||||||
logger: Logger;
|
logger: Logger;
|
||||||
runtimeSettings?: ProviderRuntimeSettings;
|
runtimeSettings?: ProviderRuntimeSettings;
|
||||||
command?: [string, ...string[]];
|
command?: [string, ...string[]];
|
||||||
|
commandsRpcType?: PiCommandsRpcType;
|
||||||
spawnProcess?: (launch: PiRuntimeLaunch) => ChildProcessWithoutNullStreams;
|
spawnProcess?: (launch: PiRuntimeLaunch) => ChildProcessWithoutNullStreams;
|
||||||
}
|
}
|
||||||
|
|
||||||
export class PiCliRuntime implements PiRuntime {
|
export class PiCliRuntime implements PiRuntime {
|
||||||
private readonly command: [string, ...string[]];
|
private readonly command: [string, ...string[]];
|
||||||
|
private readonly commandsRpcType: PiCommandsRpcType;
|
||||||
private readonly spawnProcess: (launch: PiRuntimeLaunch) => ChildProcessWithoutNullStreams;
|
private readonly spawnProcess: (launch: PiRuntimeLaunch) => ChildProcessWithoutNullStreams;
|
||||||
|
|
||||||
constructor(private readonly options: PiCliRuntimeOptions) {
|
constructor(private readonly options: PiCliRuntimeOptions) {
|
||||||
this.command = options.command ?? DEFAULT_PI_COMMAND;
|
this.command = options.command ?? DEFAULT_PI_COMMAND;
|
||||||
|
this.commandsRpcType = options.commandsRpcType ?? DEFAULT_COMMANDS_RPC_TYPE;
|
||||||
this.spawnProcess =
|
this.spawnProcess =
|
||||||
options.spawnProcess ??
|
options.spawnProcess ??
|
||||||
((launch) => {
|
((launch) => {
|
||||||
@@ -77,7 +82,12 @@ export class PiCliRuntime implements PiRuntime {
|
|||||||
runtimeSettings: this.options.runtimeSettings,
|
runtimeSettings: this.options.runtimeSettings,
|
||||||
session: input,
|
session: input,
|
||||||
});
|
});
|
||||||
return new PiCliRuntimeSession(launch, this.spawnProcess(launch), this.options.logger);
|
return new PiCliRuntimeSession(
|
||||||
|
launch,
|
||||||
|
this.spawnProcess(launch),
|
||||||
|
this.options.logger,
|
||||||
|
this.commandsRpcType,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -93,6 +103,7 @@ class PiCliRuntimeSession implements PiRuntimeSession {
|
|||||||
_launch: PiRuntimeLaunch,
|
_launch: PiRuntimeLaunch,
|
||||||
private readonly child: ChildProcessWithoutNullStreams,
|
private readonly child: ChildProcessWithoutNullStreams,
|
||||||
private readonly logger: Logger,
|
private readonly logger: Logger,
|
||||||
|
private readonly commandsRpcType: PiCommandsRpcType,
|
||||||
) {
|
) {
|
||||||
child.stdout.on("data", (chunk) => {
|
child.stdout.on("data", (chunk) => {
|
||||||
this.handleStdoutChunk(chunk.toString());
|
this.handleStdoutChunk(chunk.toString());
|
||||||
@@ -173,7 +184,7 @@ class PiCliRuntimeSession implements PiRuntimeSession {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async getCommands(): Promise<PiRpcSlashCommand[]> {
|
async getCommands(): Promise<PiRpcSlashCommand[]> {
|
||||||
const data = (await this.request({ type: "get_commands" })) as {
|
const data = (await this.request({ type: this.commandsRpcType })) as {
|
||||||
commands?: PiRpcSlashCommand[];
|
commands?: PiRpcSlashCommand[];
|
||||||
};
|
};
|
||||||
return data.commands ?? [];
|
return data.commands ?? [];
|
||||||
|
|||||||
@@ -110,6 +110,8 @@ export interface PiRpcSlashCommand {
|
|||||||
sourceInfo?: Record<string, unknown>;
|
sourceInfo?: Record<string, unknown>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type PiCommandsRpcType = "get_commands" | "get_available_commands";
|
||||||
|
|
||||||
export type PiRpcCommand =
|
export type PiRpcCommand =
|
||||||
| { id?: string; type: "prompt"; message: string; images?: PiImageContent[] }
|
| { id?: string; type: "prompt"; message: string; images?: PiImageContent[] }
|
||||||
| { id?: string; type: "compact"; customInstructions?: string }
|
| { id?: string; type: "compact"; customInstructions?: string }
|
||||||
@@ -121,7 +123,7 @@ export type PiRpcCommand =
|
|||||||
| { id?: string; type: "set_model"; provider: string; modelId: string }
|
| { id?: string; type: "set_model"; provider: string; modelId: string }
|
||||||
| { id?: string; type: "set_thinking_level"; level: PiThinkingLevel }
|
| { id?: string; type: "set_thinking_level"; level: PiThinkingLevel }
|
||||||
| { id?: string; type: "get_session_stats" }
|
| { id?: string; type: "get_session_stats" }
|
||||||
| { id?: string; type: "get_commands" };
|
| { id?: string; type: PiCommandsRpcType };
|
||||||
|
|
||||||
export interface PiRpcResponse {
|
export interface PiRpcResponse {
|
||||||
id?: string;
|
id?: string;
|
||||||
|
|||||||
Reference in New Issue
Block a user