fix(coding-agent): disambiguate duplicate slash commands, fixes #1061
This commit is contained in:
@@ -345,9 +345,10 @@ describe("ExtensionRunner", () => {
|
||||
|
||||
expect(commands.length).toBe(2);
|
||||
expect(commands.map((c) => c.name).sort()).toEqual(["cmd-a", "cmd-b"]);
|
||||
expect(commands.map((c) => c.invocationName).sort()).toEqual(["cmd-a", "cmd-b"]);
|
||||
});
|
||||
|
||||
it("gets command by name", async () => {
|
||||
it("gets command by invocation name", async () => {
|
||||
const cmdCode = `
|
||||
export default function(pi) {
|
||||
pi.registerCommand("my-cmd", {
|
||||
@@ -364,13 +365,14 @@ describe("ExtensionRunner", () => {
|
||||
const cmd = runner.getCommand("my-cmd");
|
||||
expect(cmd).toBeDefined();
|
||||
expect(cmd?.name).toBe("my-cmd");
|
||||
expect(cmd?.invocationName).toBe("my-cmd");
|
||||
expect(cmd?.description).toBe("My command");
|
||||
|
||||
const missing = runner.getCommand("not-exists");
|
||||
expect(missing).toBeUndefined();
|
||||
});
|
||||
|
||||
it("filters out duplicate extension commands", async () => {
|
||||
it("suffixes duplicate extension commands in insertion order", async () => {
|
||||
const cmdCode = (description: string) => `
|
||||
export default function(pi) {
|
||||
pi.registerCommand("shared-cmd", {
|
||||
@@ -382,22 +384,18 @@ describe("ExtensionRunner", () => {
|
||||
fs.writeFileSync(path.join(extensionsDir, "cmd-a.ts"), cmdCode("First command"));
|
||||
fs.writeFileSync(path.join(extensionsDir, "cmd-b.ts"), cmdCode("Second command"));
|
||||
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
const result = await discoverAndLoadExtensions([], tempDir, tempDir);
|
||||
const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
|
||||
const commands = runner.getRegisteredCommands();
|
||||
const diagnostics = runner.getCommandDiagnostics();
|
||||
|
||||
expect(commands).toHaveLength(1);
|
||||
expect(commands[0]?.name).toBe("shared-cmd");
|
||||
expect(commands[0]?.description).toBe("First command");
|
||||
|
||||
expect(diagnostics).toHaveLength(1);
|
||||
expect(diagnostics[0]?.path).toEqual(path.join(extensionsDir, "cmd-b.ts"));
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("conflicts with"));
|
||||
warnSpy.mockRestore();
|
||||
expect(commands).toHaveLength(2);
|
||||
expect(commands.map((command) => command.name)).toEqual(["shared-cmd", "shared-cmd"]);
|
||||
expect(commands.map((command) => command.invocationName)).toEqual(["shared-cmd:1", "shared-cmd:2"]);
|
||||
expect(commands.map((command) => command.description)).toEqual(["First command", "Second command"]);
|
||||
expect(diagnostics).toEqual([]);
|
||||
expect(runner.getCommand("shared-cmd:1")?.description).toBe("First command");
|
||||
expect(runner.getCommand("shared-cmd:2")?.description).toBe("Second command");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -195,7 +195,7 @@ Project skill`,
|
||||
|
||||
const extensionsResult = loader.getExtensions();
|
||||
expect(extensionsResult.extensions).toHaveLength(2);
|
||||
expect(extensionsResult.errors.some((e) => e.error.includes('Command "/deploy" conflicts'))).toBe(true);
|
||||
expect(extensionsResult.errors.some((e) => e.error.includes('Command "/deploy" conflicts'))).toBe(false);
|
||||
|
||||
const sessionManager = SessionManager.inMemory();
|
||||
const authStorage = AuthStorage.create(join(tempDir, "auth.json"));
|
||||
@@ -208,12 +208,18 @@ Project skill`,
|
||||
modelRegistry,
|
||||
);
|
||||
|
||||
expect(runner.getCommand("deploy")?.description).toBe("project deploy");
|
||||
expect(runner.getCommand("deploy:1")?.description).toBe("project deploy");
|
||||
expect(runner.getCommand("deploy:2")?.description).toBe("user deploy");
|
||||
expect(runner.getCommand("project-only")?.description).toBe("project only");
|
||||
expect(runner.getCommand("user-only")?.description).toBe("user only");
|
||||
|
||||
const commandNames = runner.getRegisteredCommands().map((c) => c.name);
|
||||
expect(commandNames.filter((name) => name === "deploy")).toHaveLength(1);
|
||||
const commands = runner.getRegisteredCommands();
|
||||
expect(commands.map((command) => command.invocationName)).toEqual([
|
||||
"deploy:1",
|
||||
"project-only",
|
||||
"deploy:2",
|
||||
"user-only",
|
||||
]);
|
||||
});
|
||||
|
||||
it("should honor overrides for auto-discovered resources", async () => {
|
||||
@@ -551,7 +557,8 @@ export default function(pi: ExtensionAPI) {
|
||||
modelRegistry,
|
||||
);
|
||||
|
||||
expect(runner.getCommand("deploy")?.description).toBe("explicit command");
|
||||
expect(runner.getCommand("deploy:1")?.description).toBe("explicit command");
|
||||
expect(runner.getCommand("deploy:2")?.description).toBe("global command");
|
||||
expect(runner.getToolDefinition("duplicate-tool")?.description).toBe("explicit tool");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -7,7 +7,7 @@ import type { AgentTool } from "@mariozechner/pi-agent-core";
|
||||
import type { AssistantMessage } from "@mariozechner/pi-ai";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createHarness, type Harness } from "./test-harness.js";
|
||||
import { createHarness, createHarnessWithExtensions, type Harness } from "./test-harness.js";
|
||||
|
||||
describe("test harness", () => {
|
||||
let harness: Harness;
|
||||
@@ -257,6 +257,58 @@ describe("test harness", () => {
|
||||
expect(firstText).toBeLessThan(firstToolcall);
|
||||
});
|
||||
|
||||
it("loads inline extension factories and disambiguates duplicate commands", async () => {
|
||||
const calls: string[] = [];
|
||||
|
||||
harness = await createHarnessWithExtensions({
|
||||
extensionFactories: [
|
||||
{
|
||||
path: "<alpha>",
|
||||
factory: (pi) => {
|
||||
pi.registerCommand("shared-cmd", {
|
||||
description: "Alpha command",
|
||||
handler: async (args) => {
|
||||
calls.push(`alpha:${args}`);
|
||||
},
|
||||
});
|
||||
},
|
||||
},
|
||||
{
|
||||
path: "<beta>",
|
||||
factory: (pi) => {
|
||||
pi.registerCommand("shared-cmd", {
|
||||
description: "Beta command",
|
||||
handler: async (args) => {
|
||||
calls.push(`beta:${args}`);
|
||||
},
|
||||
});
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
const runner = harness.session.extensionRunner;
|
||||
expect(runner).toBeDefined();
|
||||
|
||||
const commands = runner!.getRegisteredCommands();
|
||||
expect(
|
||||
commands.map((command) => ({
|
||||
name: command.name,
|
||||
invocationName: command.invocationName,
|
||||
description: command.description,
|
||||
path: command.sourceInfo.path,
|
||||
})),
|
||||
).toEqual([
|
||||
{ name: "shared-cmd", invocationName: "shared-cmd:1", description: "Alpha command", path: "<alpha>" },
|
||||
{ name: "shared-cmd", invocationName: "shared-cmd:2", description: "Beta command", path: "<beta>" },
|
||||
]);
|
||||
|
||||
await runner!.getCommand("shared-cmd:1")?.handler("first", runner!.createCommandContext());
|
||||
await runner!.getCommand("shared-cmd:2")?.handler("second", runner!.createCommandContext());
|
||||
|
||||
expect(calls).toEqual(["alpha:first", "beta:second"]);
|
||||
});
|
||||
|
||||
it("session persistence works", async () => {
|
||||
harness = createHarness({ responses: ["persisted"] });
|
||||
|
||||
|
||||
@@ -32,7 +32,12 @@ import { ModelRegistry } from "../src/core/model-registry.js";
|
||||
import { SessionManager } from "../src/core/session-manager.js";
|
||||
import type { Settings } from "../src/core/settings-manager.js";
|
||||
import { SettingsManager } from "../src/core/settings-manager.js";
|
||||
import { createTestResourceLoader } from "./utilities.js";
|
||||
import type { ExtensionFactory, ResourceLoader } from "../src/index.js";
|
||||
import {
|
||||
type CreateTestExtensionsResultInput,
|
||||
createTestExtensionsResult,
|
||||
createTestResourceLoader,
|
||||
} from "./utilities.js";
|
||||
|
||||
// ============================================================================
|
||||
// Faux model
|
||||
@@ -327,6 +332,10 @@ export interface HarnessOptions {
|
||||
tools?: AgentTool[];
|
||||
/** Base tools override (replaces built-in read/bash/edit/write). */
|
||||
baseToolsOverride?: Record<string, AgentTool>;
|
||||
/** Optional resource loader override. */
|
||||
resourceLoader?: ResourceLoader;
|
||||
/** Inline extensions to load into the session resource loader. */
|
||||
extensionFactories?: Array<ExtensionFactory | CreateTestExtensionsResultInput>;
|
||||
}
|
||||
|
||||
export interface Harness {
|
||||
@@ -346,10 +355,17 @@ export interface Harness {
|
||||
cleanup: () => void;
|
||||
}
|
||||
|
||||
export function createHarness(options: HarnessOptions = {}): Harness {
|
||||
function createTempDir(): string {
|
||||
const tempDir = join(tmpdir(), `pi-harness-${Date.now()}-${Math.random().toString(36).slice(2)}`);
|
||||
mkdirSync(tempDir, { recursive: true });
|
||||
return tempDir;
|
||||
}
|
||||
|
||||
function createHarnessWithResourceLoader(
|
||||
options: HarnessOptions,
|
||||
resourceLoader: ResourceLoader,
|
||||
tempDir: string,
|
||||
): Harness {
|
||||
const baseModel = options.model ?? fauxModel;
|
||||
const model: Model<any> = options.contextWindow ? { ...baseModel, contextWindow: options.contextWindow } : baseModel;
|
||||
|
||||
@@ -382,7 +398,7 @@ export function createHarness(options: HarnessOptions = {}): Harness {
|
||||
settingsManager,
|
||||
cwd: tempDir,
|
||||
modelRegistry,
|
||||
resourceLoader: createTestResourceLoader(),
|
||||
resourceLoader,
|
||||
baseToolsOverride: options.baseToolsOverride,
|
||||
});
|
||||
|
||||
@@ -412,3 +428,19 @@ export function createHarness(options: HarnessOptions = {}): Harness {
|
||||
cleanup,
|
||||
};
|
||||
}
|
||||
|
||||
export function createHarness(options: HarnessOptions = {}): Harness {
|
||||
if (options.extensionFactories?.length) {
|
||||
throw new Error("createHarness does not support extensionFactories. Use createHarnessWithExtensions().");
|
||||
}
|
||||
|
||||
const tempDir = createTempDir();
|
||||
return createHarnessWithResourceLoader(options, options.resourceLoader ?? createTestResourceLoader(), tempDir);
|
||||
}
|
||||
|
||||
export async function createHarnessWithExtensions(options: HarnessOptions = {}): Promise<Harness> {
|
||||
const tempDir = createTempDir();
|
||||
const extensionsResult = await createTestExtensionsResult(options.extensionFactories ?? [], tempDir);
|
||||
const resourceLoader = options.resourceLoader ?? createTestResourceLoader({ extensionsResult });
|
||||
return createHarnessWithResourceLoader(options, resourceLoader, tempDir);
|
||||
}
|
||||
|
||||
@@ -10,7 +10,9 @@ import { getModel, type OAuthCredentials, type OAuthProvider } from "@mariozechn
|
||||
import { getOAuthApiKey } from "@mariozechner/pi-ai/oauth";
|
||||
import { AgentSession } from "../src/core/agent-session.js";
|
||||
import { AuthStorage } from "../src/core/auth-storage.js";
|
||||
import { createExtensionRuntime } from "../src/core/extensions/loader.js";
|
||||
import { createEventBus } from "../src/core/event-bus.js";
|
||||
import type { Extension, ExtensionFactory, LoadExtensionsResult } from "../src/core/extensions/index.js";
|
||||
import { createExtensionRuntime, loadExtensionFromFactory } from "../src/core/extensions/loader.js";
|
||||
import { ModelRegistry } from "../src/core/model-registry.js";
|
||||
import type { ResourceLoader } from "../src/core/resource-loader.js";
|
||||
import { SessionManager } from "../src/core/session-manager.js";
|
||||
@@ -175,9 +177,46 @@ export interface TestSessionContext {
|
||||
cleanup: () => void;
|
||||
}
|
||||
|
||||
export function createTestResourceLoader(): ResourceLoader {
|
||||
export interface CreateTestExtensionsResultInput {
|
||||
factory: ExtensionFactory;
|
||||
path?: string;
|
||||
}
|
||||
|
||||
export async function createTestExtensionsResult(
|
||||
inputs: Array<ExtensionFactory | CreateTestExtensionsResultInput>,
|
||||
cwd = process.cwd(),
|
||||
): Promise<LoadExtensionsResult> {
|
||||
const runtime = createExtensionRuntime();
|
||||
const eventBus = createEventBus();
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
for (const [index, input] of inputs.entries()) {
|
||||
const factory = typeof input === "function" ? input : input.factory;
|
||||
const extensionPath =
|
||||
typeof input === "function" ? `<inline:${index + 1}>` : (input.path ?? `<inline:${index + 1}>`);
|
||||
extensions.push(await loadExtensionFromFactory(factory, cwd, eventBus, runtime, extensionPath));
|
||||
}
|
||||
|
||||
return {
|
||||
getExtensions: () => ({ extensions: [], errors: [], runtime: createExtensionRuntime() }),
|
||||
extensions,
|
||||
errors: [],
|
||||
runtime,
|
||||
};
|
||||
}
|
||||
|
||||
export interface CreateTestResourceLoaderOptions {
|
||||
extensionsResult?: LoadExtensionsResult;
|
||||
}
|
||||
|
||||
export function createTestResourceLoader(options: CreateTestResourceLoaderOptions = {}): ResourceLoader {
|
||||
const extensionsResult = options.extensionsResult ?? {
|
||||
extensions: [],
|
||||
errors: [],
|
||||
runtime: createExtensionRuntime(),
|
||||
};
|
||||
|
||||
return {
|
||||
getExtensions: () => extensionsResult,
|
||||
getSkills: () => ({ skills: [], diagnostics: [] }),
|
||||
getPrompts: () => ({ prompts: [], diagnostics: [] }),
|
||||
getThemes: () => ({ themes: [], diagnostics: [] }),
|
||||
|
||||
Reference in New Issue
Block a user