smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): identify user-invoked skills and expose skill directory

Browse Source 
User-invoked skills (typed /skill:, steered, follow-up, interrupted/
resumed via compaction, ACP, RPC) only appended a bare "Skill: <path>"
line, so the model neither learned the user had invoked that specific
skill nor where the skill directory was. Relative paths in skill bodies
(scripts/, templates/) could not be resolved.

Route all user-invoked paths through a self-identifying, baseDir-aware
prompt template; keep hidden autoload skills on the minimal non-user
format. Interactive skillCommands now carries the loaded Skill object
instead of a bare path so baseDir flows through without reconstruction.
The invocation kind defaults to "user" to keep buildSkillPromptMessage
source-compatible.

Op: correct
Restores: spec:user-invoked-skill-prompt-self-identifies-and-exposes-skill-directory
This commit is contained in:
metaphorics
2026-06-30 23:11:05 +09:00
parent 5bc68f57cd
commit fa64097992
15 changed files with 163 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/test/acp-agent.test.ts
+10 -5
View File
@@ -1410,7 +1410,8 @@ describe("ACP agent", () => {
it("executes skill commands through custom skill messages", async () => {
const harness = await createHarness();
const created = await harness.agent.newSession({ cwd: harness.cwdA, mcpServers: [] });
const session = harness.findSession(created.sessionId)!;
const session = harness.findSession(created.sessionId);
if (!session) throw new Error("expected ACP session to exist after newSession");
const skillDir = path.join(harness.cwdA, ".skills", "sample");
const skillPath = path.join(skillDir, "SKILL.md");
await fs.promises.mkdir(skillDir, { recursive: true });
@@ -1433,10 +1434,14 @@ describe("ACP agent", () => {
expect(session.promptCalls).toEqual([]);
expect(session.customMessages).toHaveLength(1);
expect(session.customMessages[0]!.customType).toBe("skill-prompt");
expect(session.customMessages[0]!.content).toContain("# Sample\nDo work.");
expect(session.customMessages[0]!.content).toContain(`Skill: ${skillPath}`);
expect(session.customMessages[0]!.content).toContain("User: extra context");
const customMessage = session.customMessages[0];
if (!customMessage) throw new Error("expected ACP skill prompt custom message");
expect(customMessage.customType).toBe("skill-prompt");
expect(customMessage.content).toContain("# Sample\nDo work.");
expect(customMessage.content).toContain('The user has invoked the "sample" skill');
expect(customMessage.content).toContain(`[Skill directory: ${skillDir}]`);
expect(customMessage.content).toMatch(/[Rr]esolve any relative paths/);
expect(customMessage.content).toContain("User: extra context");
harness.abortController.abort();
await Bun.sleep(0);
packages/coding-agent/test/input-controller-skill-queue.test.ts
+23 -12
View File
@@ -8,10 +8,11 @@
import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "bun:test";
import * as path from "node:path";
import { Agent } from "@oh-my-pi/pi-agent-core";
import type { ImageContent } from "@oh-my-pi/pi-ai";
import type { ImageContent, TextContent } from "@oh-my-pi/pi-ai";
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import type { Skill } from "@oh-my-pi/pi-coding-agent/extensibility/skills";
import { EventController } from "@oh-my-pi/pi-coding-agent/modes/controllers/event-controller";
import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller";
import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
@@ -40,7 +41,7 @@ type PromptCustomMessage = Mock<
(
message: {
customType?: string;
content?: string | unknown[];
content?: string | (TextContent | ImageContent)[];
display?: boolean;
attribution?: string;
details: SkillPromptDetails;
@@ -49,14 +50,14 @@ type PromptCustomMessage = Mock<
) => Promise<void>
>;
async function writeSkillFile(dir: string, skillName: string, body: string): Promise<string> {
async function writeSkillFile(dir: string, skillName: string, body: string): Promise<Skill> {
const skillPath = path.join(dir, `${skillName}.md`);
await Bun.write(skillPath, `---\nname: ${skillName}\n---\n${body}\n`);
return skillPath;
return { name: skillName, description: "", filePath: skillPath, baseDir: dir, source: "test" };
}
function createStubInputControllerContext(opts: {
skillCommands: Map<string, string>;
skillCommands: Map<string, Skill>;
isStreaming: boolean;
isCompacting?: boolean;
}) {
@@ -132,12 +133,12 @@ function createStubInputControllerContext(opts: {
describe("InputController skill queue chip metadata", () => {
let tempDir: TempDir;
let skillCommands: Map<string, string>;
let skillCommands: Map<string, Skill>;
beforeEach(async () => {
tempDir = TempDir.createSync("@pi-skill-queue-stub-");
const skillPath = await writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
skillCommands = new Map<string, string>([["skill:test-skill", skillPath]]);
const skill = await writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
skillCommands = new Map<string, Skill>([["skill:test-skill", skill]]);
});
afterEach(() => {
@@ -260,7 +261,7 @@ describe("InputController skill queue chip metadata", () => {
describe("compaction skill re-invocation", () => {
let tempDir: TempDir;
let skillCommands: Map<string, string>;
let skillCommands: Map<string, Skill>;
function firstPromptCustomCall(promptCustomMessage: PromptCustomMessage) {
const call = promptCustomMessage.mock.calls[0];
@@ -299,8 +300,8 @@ describe("compaction skill re-invocation", () => {
beforeEach(async () => {
tempDir = TempDir.createSync("@pi-skill-compaction-stub-");
const skillPath = await writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
skillCommands = new Map<string, string>([["skill:test-skill", skillPath]]);
const skill = await writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
skillCommands = new Map<string, Skill>([["skill:test-skill", skill]]);
});
afterEach(() => {
@@ -323,7 +324,17 @@ describe("compaction skill re-invocation", () => {
if (!Array.isArray(message.content)) {
throw new Error("expected queued skill prompt to preserve image content blocks");
}
expect(message.content[0]).toMatchObject({ type: "text", text: expect.stringContaining("Do the thing.") });
const renderedText = message.content[0];
if (renderedText?.type !== "text") {
throw new Error("expected first content block to be rendered skill text");
}
// Bug fix contract: a re-invoked user skill identifies itself and exposes its
// skill directory so relative skill paths resolve after compaction.
expect(renderedText.text).toContain("Do the thing.");
expect(renderedText.text).toContain('The user has invoked the "test-skill" skill');
expect(renderedText.text).toContain(`[Skill directory: ${tempDir.path()}]`);
expect(renderedText.text).toMatch(/[Rr]esolve any relative paths/);
expect(renderedText.text).toContain("User: arg1 arg2");
expect(message.content[1]).toEqual(image);
expect(message.details).toMatchObject({ name: "test-skill", args: "arg1 arg2", lineCount: 1 });
expect(options).toEqual({
packages/coding-agent/test/rpc-skill-command.test.ts
+3
View File
@@ -33,6 +33,9 @@ describe("tryRunRpcSkillCommand", () => {
expect(handled).toEqual({ agentInvoked: true });
expect(message?.customType).toBe(SKILL_PROMPT_MESSAGE_TYPE);
expect(message?.content).toContain("Review the supplied code carefully.");
expect(message?.content).toContain('The user has invoked the "reviewer" skill');
expect(message?.content).toContain(`[Skill directory: ${dir}]`);
expect(message?.content).toMatch(/[Rr]esolve any relative paths/);
expect(message?.content).toContain("User: focus on risks");
expect(message?.display).toBe(true);
expect(message?.attribution).toBe("user");
packages/coding-agent/test/skill-prompt-message.test.ts
+60
View File
@@ -0,0 +1,60 @@
import { describe, expect, test } from "bun:test";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { buildSkillPromptMessage, type Skill } from "@oh-my-pi/pi-coding-agent/extensibility/skills";
import { removeWithRetries, Snowflake } from "@oh-my-pi/pi-utils";
async function createSkill(body: string): Promise<{ dir: string; skill: Skill }> {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), `omp-skill-prompt-${Snowflake.next()}-`));
const filePath = path.join(dir, "SKILL.md");
await Bun.write(filePath, `---\nname: reviewer\ndescription: Review code\n---\n\n${body}\n`);
return {
dir,
skill: {
name: "reviewer",
description: "Review code",
filePath,
baseDir: dir,
source: "test",
},
};
}
describe("buildSkillPromptMessage", () => {
test("defaults public skill prompt rendering to user-invoked bug-fix directory guidance", async () => {
const { dir, skill } = await createSkill("Review the supplied code carefully.");
try {
const built = await buildSkillPromptMessage(skill, "focus on risks");
expect(built.message).toContain("Review the supplied code carefully.");
expect(built.message).toContain('The user has invoked the "reviewer" skill');
expect(built.message).toContain(`[Skill directory: ${dir}]`);
expect(built.message).toMatch(/[Rr]esolve any relative paths/);
expect(built.message).toContain("User: focus on risks");
expect(built.details).toMatchObject({
name: "reviewer",
path: skill.filePath,
args: "focus on risks",
lineCount: 1,
});
} finally {
await removeWithRetries(dir);
}
});
test("keeps autoload skills on non-user minimal framing", async () => {
const { dir, skill } = await createSkill("Review silently loaded context.");
try {
const built = await buildSkillPromptMessage(skill, "", "autoload");
expect(built.message).toContain("Review silently loaded context.");
expect(built.message).toContain(`Skill: ${skill.filePath}`);
expect(built.message).not.toContain("The user has invoked");
expect(built.message).not.toContain("[Skill directory:");
expect(built.details).toMatchObject({ name: "reviewer", path: skill.filePath, lineCount: 1 });
} finally {
await removeWithRetries(dir);
}
});
});