diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts index 6800a7cef0..2ec361ddae 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.test.ts @@ -2054,6 +2054,125 @@ describe("CodexAppServerAgent", () => { ]); }); + it("injects _meta.localSkillContext in place of the bare skill command, echo unchanged", async () => { + const stub = makeStubRpc({ + "thread/start": { thread: { id: "t" } }, + "turn/start": { turn: { id: "turn_1" } }, + }); + const { client, sessionUpdates } = makeFakeClient(); + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/x/codex" }, + rpcFactory: stub.factory, + }); + + await agent.newSession({ cwd: "/r" } as unknown as NewSessionRequest); + const done = agent.prompt({ + sessionId: "t", + prompt: [{ type: "text", text: "/my-skill do the thing" }], + _meta: { + localSkillContext: "BEGIN SKILL my-skill ... END SKILL", + localSkillName: "my-skill", + }, + } as unknown as PromptRequest); + stub.emit("turn/completed", { turn: { status: "completed" } }); + await done; + + const turnStart = stub.requests.find((r) => r.method === "turn/start"); + expect( + (turnStart?.params as { input: Array<{ text?: string }> }).input, + ).toEqual([ + { + type: "text", + text: "BEGIN SKILL my-skill ... END SKILL", + text_elements: [], + }, + ]); + // The echoed user turn still shows what the user actually typed. + const echoes = (sessionUpdates as any[]).filter( + (u) => u.update?.sessionUpdate === "user_message_chunk", + ); + expect(echoes).toEqual([ + { + sessionId: "t", + update: { + sessionUpdate: "user_message_chunk", + content: { type: "text", text: "/my-skill do the thing" }, + }, + }, + ]); + }); + + it("orders prContext before localSkillContext and keeps non-command chunks", async () => { + const stub = makeStubRpc({ + "thread/start": { thread: { id: "t" } }, + "turn/start": { turn: { id: "turn_1" } }, + }); + const { client } = makeFakeClient(); + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/x/codex" }, + rpcFactory: stub.factory, + }); + + await agent.newSession({ cwd: "/r" } as unknown as NewSessionRequest); + const done = agent.prompt({ + sessionId: "t", + prompt: [ + { type: "text", text: "/my-skill" }, + { type: "text", text: "also check the README" }, + ], + _meta: { + prContext: "PR #123 is open.", + localSkillContext: "SKILL DEFINITION", + localSkillName: "my-skill", + }, + } as unknown as PromptRequest); + stub.emit("turn/completed", { turn: { status: "completed" } }); + await done; + + const turnStart = stub.requests.find((r) => r.method === "turn/start"); + expect( + (turnStart?.params as { input: Array<{ text?: string }> }).input, + ).toEqual([ + { type: "text", text: "PR #123 is open.", text_elements: [] }, + { type: "text", text: "SKILL DEFINITION", text_elements: [] }, + { type: "text", text: "also check the README", text_elements: [] }, + ]); + }); + + it("injects localSkillContext without a skill name and keeps the message text", async () => { + const stub = makeStubRpc({ + "thread/start": { thread: { id: "t" } }, + "turn/start": { turn: { id: "turn_1" } }, + }); + const { client } = makeFakeClient(); + const agent = new CodexAppServerAgent(client, { + processOptions: { binaryPath: "/x/codex" }, + rpcFactory: stub.factory, + }); + + await agent.newSession({ cwd: "/r" } as unknown as NewSessionRequest); + // a mid-message mention has no command chunk to strip, so no localSkillName + const done = agent.prompt({ + sessionId: "t", + prompt: [{ type: "text", text: "please use /my-skill for this" }], + _meta: { localSkillContext: "ATTACHED SKILLS CONTEXT" }, + } as unknown as PromptRequest); + stub.emit("turn/completed", { turn: { status: "completed" } }); + await done; + + const turnStart = stub.requests.find((r) => r.method === "turn/start"); + expect( + (turnStart?.params as { input: Array<{ text?: string }> }).input, + ).toEqual([ + { type: "text", text: "ATTACHED SKILLS CONTEXT", text_elements: [] }, + { + type: "text", + text: "please use /my-skill for this", + text_elements: [], + }, + ]); + }); + it("echoes an image-only user turn as a user_message_chunk", async () => { const stub = makeStubRpc({ "thread/start": { thread: { id: "t" } }, diff --git a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts index e2209f5e05..b39035fc32 100644 --- a/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts +++ b/packages/agent/src/adapters/codex-app-server/codex-app-server-agent.ts @@ -35,6 +35,7 @@ import { emptyBaseline, estimateTokens, } from "../claude/context-breakdown"; +import { isLocalSkillCommandChunk } from "../local-skill"; import { AppServerClient, type AppServerClientHandlers, @@ -551,12 +552,48 @@ export class CodexAppServerAgent extends BaseAcpAgent { this.planHandoffCancel?.(); // Prepend _meta.prContext (host PR-follow-up / Slack runs) to the FORWARDED prompt, // else codex cloud follow-ups lose the PR-review context. The echo omits it. - const prContext = (params._meta as { prContext?: unknown } | undefined) - ?.prContext; + const meta = params._meta as + | { + prContext?: unknown; + localSkillContext?: unknown; + localSkillName?: unknown; + } + | undefined; + const prContext = meta?.prContext; + // Inline installed local skill definitions (mirrors the Claude adapter): + // codex's skill catalog is fixed at thread start, so mid-session installs + // are invisible without this. A bare `/name` command chunk is dropped — + // the injected context already carries the user's args. + const localSkillContext = + typeof meta?.localSkillContext === "string" + ? meta.localSkillContext + : null; + const localSkillName = + typeof meta?.localSkillName === "string" ? meta.localSkillName : null; + let forwarded = params.prompt; + if (localSkillContext) { + if (localSkillName) { + let skippedLocalSkillCommand = false; + forwarded = forwarded.filter((chunk) => { + if ( + !skippedLocalSkillCommand && + isLocalSkillCommandChunk(chunk, localSkillName) + ) { + skippedLocalSkillCommand = true; + return false; + } + return true; + }); + } + forwarded = [ + { type: "text" as const, text: localSkillContext }, + ...forwarded, + ]; + } const promptBlocks = typeof prContext === "string" && prContext.length > 0 - ? [{ type: "text" as const, text: prContext }, ...params.prompt] - : params.prompt; + ? [{ type: "text" as const, text: prContext }, ...forwarded] + : forwarded; const input = toCodexInput(promptBlocks); if (input.length === 0) { // turn/start rejects empty input, so end the turn cleanly. diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index 78f98ae6e1..bf5aab9734 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -1527,6 +1527,215 @@ describe("AgentServer HTTP Mode", () => { expect(sentMeta?.localSkillName).toBe("local-test-skill"); }, 20000); + it("lists co-installed dependency skills with their paths in the skill context", async () => { + const makeBundle = (name: string, body: string) => + zipSync({ + "SKILL.md": new TextEncoder().encode( + [ + "---", + `name: ${name}`, + `description: ${name}`, + "---", + "", + body, + ].join("\n"), + ), + }); + const invokedBundle = makeBundle( + "parent-skill", + "Use /dep-skill for the review step.", + ); + const depBundle = makeBundle("dep-skill", "Dependency instructions."); + const checksumOf = (bundle: Uint8Array) => + createHash("sha256").update(Buffer.from(bundle)).digest("hex"); + + const s = createServer(); + await s.start(); + const prompt = vi.fn( + async (_params: { + prompt: ContentBlock[]; + _meta?: Record; + }) => ({ stopReason: "cancelled" }) as { stopReason: string }, + ); + const downloadArtifact = vi.fn( + async (_taskId: string, _runId: string, storagePath: string) => + exactArrayBuffer( + storagePath.includes("dep-skill") ? depBundle : invokedBundle, + ), + ); + const serverInternals = s as unknown as { + session: { clientConnection: { prompt: typeof prompt } }; + posthogAPI: { downloadArtifact: typeof downloadArtifact }; + }; + serverInternals.session.clientConnection.prompt = prompt; + serverInternals.posthogAPI.downloadArtifact = downloadArtifact; + + const makeArtifact = ( + id: string, + name: string, + bundle: Uint8Array, + ): Record => ({ + id, + name: `${name}.zip`, + type: "skill_bundle", + source: "posthog_code_skill", + storage_path: `tasks/artifacts/${name}.zip`, + content_type: "application/zip", + metadata: { + skill_name: name, + skill_source: "user", + content_sha256: checksumOf(bundle), + bundle_format: "zip", + schema_version: 1, + }, + }); + + const token = createToken(); + const response = await fetch(`http://localhost:${port}/command`, { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: "skill-command-deps", + method: "user_message", + params: { + content: "/parent-skill run it", + artifacts: [ + makeArtifact( + "skill-artifact-parent", + "parent-skill", + invokedBundle, + ), + makeArtifact("skill-artifact-dep", "dep-skill", depBundle), + ], + }, + }), + }); + + expect(response.status).toBe(200); + expect(prompt).toHaveBeenCalledOnce(); + + const sentMeta = prompt.mock.calls[0]?.[0]._meta; + const context = sentMeta?.localSkillContext as string; + expect(sentMeta?.localSkillName).toBe("parent-skill"); + expect(context).toContain('local skill "/parent-skill"'); + expect(context).toContain("Other local skills installed for this run"); + expect(context).toMatch(/- \/dep-skill: \S*dep-skill/); + }, 20000); + + it("announces mid-message skill mentions via localSkillContext without a skill name", async () => { + const makeBundle = (name: string, body: string) => + zipSync({ + "SKILL.md": new TextEncoder().encode( + [ + "---", + `name: ${name}`, + `description: ${name}`, + "---", + "", + body, + ].join("\n"), + ), + }); + const mentionedBundle = makeBundle( + "mentioned-skill", + "MENTIONED_SKILL_MARKER instructions.", + ); + const prefixBundle = makeBundle("mentioned", "PREFIX_SKILL_MARKER body."); + const checksumOf = (bundle: Uint8Array) => + createHash("sha256").update(Buffer.from(bundle)).digest("hex"); + + const s = createServer(); + await s.start(); + const prompt = vi.fn( + async (_params: { + prompt: ContentBlock[]; + _meta?: Record; + }) => ({ stopReason: "cancelled" }) as { stopReason: string }, + ); + const downloadArtifact = vi.fn( + async (_taskId: string, _runId: string, storagePath: string) => + exactArrayBuffer( + storagePath.includes("mentioned-skill") + ? mentionedBundle + : prefixBundle, + ), + ); + const serverInternals = s as unknown as { + session: { clientConnection: { prompt: typeof prompt } }; + posthogAPI: { downloadArtifact: typeof downloadArtifact }; + }; + serverInternals.session.clientConnection.prompt = prompt; + serverInternals.posthogAPI.downloadArtifact = downloadArtifact; + + const makeArtifact = ( + id: string, + name: string, + bundle: Uint8Array, + ): Record => ({ + id, + name: `${name}.zip`, + type: "skill_bundle", + source: "posthog_code_skill", + storage_path: `tasks/artifacts/${name}.zip`, + content_type: "application/zip", + metadata: { + skill_name: name, + skill_source: "user", + content_sha256: checksumOf(bundle), + bundle_format: "zip", + schema_version: 1, + }, + }); + + const token = createToken(); + const response = await fetch(`http://localhost:${port}/command`, { + method: "POST", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: "skill-mid-message", + method: "user_message", + params: { + content: "please use /mentioned-skill on the diff", + artifacts: [ + makeArtifact( + "skill-artifact-mentioned", + "mentioned-skill", + mentionedBundle, + ), + makeArtifact("skill-artifact-prefix", "mentioned", prefixBundle), + ], + }, + }), + }); + + expect(response.status).toBe(200); + expect(prompt).toHaveBeenCalledOnce(); + + const sentPrompt = prompt.mock.calls[0]?.[0].prompt; + const sentMeta = prompt.mock.calls[0]?.[0]._meta; + const context = sentMeta?.localSkillContext as string; + // not a bare invocation, so nothing to strip and no localSkillName + expect(sentMeta?.localSkillName).toBeUndefined(); + expect(context).toContain("MENTIONED_SKILL_MARKER"); + // "mentioned" is a prefix of "/mentioned-skill" but was not itself + // mentioned: listed by path, not inlined + expect(context).not.toContain("PREFIX_SKILL_MARKER"); + expect(context).toMatch(/- \/mentioned: \S*mentioned/); + const sentText = sentPrompt?.find( + (block): block is Extract => + block.type === "text", + )?.text; + expect(sentText).toBe("please use /mentioned-skill on the diff"); + }, 20000); + it("ignores a redelivered user_message whose messageId was already accepted", async () => { const s = createServer(); await s.start(); diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 337cc43710..1053e4c2e2 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -284,7 +284,8 @@ function hiddenTextBlock(text: string): ContentBlock { } interface LocalSkillPromptContext { - skillName: string; + /** Set when the message is a bare `/skill` invocation the adapter should strip. */ + skillName?: string; context: string; } @@ -2208,7 +2209,9 @@ export class AgentServer { ? { meta: { localSkillContext: localSkillContext.context, - localSkillName: localSkillContext.skillName, + ...(localSkillContext.skillName + ? { localSkillName: localSkillContext.skillName } + : {}), } satisfies Record, } : {}), @@ -2313,40 +2316,121 @@ export class AgentServer { (block): block is Extract => block.type === "text" && block.text.trim().length > 0, ); - if (textBlockIndex === -1) { - return null; - } + const textBlock = + textBlockIndex === -1 ? null : contentBlocks[textBlockIndex]; + const invocation = + textBlock?.type === "text" + ? this.parseLocalSkillInvocation(textBlock.text) + : null; - const textBlock = contentBlocks[textBlockIndex]; - if (textBlock.type !== "text") { - return null; + if (invocation) { + const hasMatchingArtifact = artifacts.some( + (artifact) => + artifact.type === "skill_bundle" && + artifact.metadata?.skill_name === invocation.skillName, + ); + const installedSkill = hasMatchingArtifact + ? this.installedSkillBundleInfo.get( + this.getInstalledSkillBundleInfoKey(runId, invocation.skillName), + ) + : undefined; + if (installedSkill) { + return { + skillName: invocation.skillName, + context: this.buildInstalledSkillPrompt( + installedSkill, + invocation.args, + this.getCoInstalledSkillBundles(runId, invocation.skillName), + ), + }; + } } - const invocation = this.parseLocalSkillInvocation(textBlock.text); - if (!invocation) { - return null; - } + const messageText = contentBlocks + .filter( + (block): block is Extract => + block.type === "text", + ) + .map((block) => block.text) + .join("\n"); + return this.buildAttachedSkillsPromptContext(runId, artifacts, messageText); + } - const hasMatchingArtifact = artifacts.some( - (artifact) => - artifact.type === "skill_bundle" && - artifact.metadata?.skill_name === invocation.skillName, - ); - if (!hasMatchingArtifact) { + /** + * Fallback for messages that install skill bundles without being a bare + * `/skill` invocation: a running session can't discover mid-session + * installs, so skills named in the message get their definition inlined + * and the rest are listed with their paths. + */ + private buildAttachedSkillsPromptContext( + runId: string, + artifacts: TaskRunArtifact[], + messageText: string, + ): LocalSkillPromptContext | null { + const installed = artifacts + .filter((artifact) => artifact.type === "skill_bundle") + .map((artifact) => artifact.metadata?.skill_name) + .filter((name): name is string => typeof name === "string") + .map((name) => + this.installedSkillBundleInfo.get( + this.getInstalledSkillBundleInfoKey(runId, name), + ), + ) + .filter((skill): skill is InstalledSkillBundle => !!skill); + if (installed.length === 0) { return null; } - const installedSkill = this.installedSkillBundleInfo.get( - this.getInstalledSkillBundleInfoKey(runId, invocation.skillName), - ); - if (!installedSkill) { - return null; + const mentioned = installed.filter((skill) => { + // token-boundary match so "/foo" never matches inside "/foobar" + const escaped = skill.skillName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp( + `(^|[\\s(\`"'\\[])/${escaped}(?![A-Za-z0-9_/-])`, + "m", + ).test(messageText); + }); + const unmentioned = installed.filter((skill) => !mentioned.includes(skill)); + + const sections: string[] = [ + "The user's message references local skills that are now installed for this run. Apply a skill's instructions when the message calls for it.", + ]; + for (const skill of mentioned) { + sections.push( + "", + `--- BEGIN LOCAL SKILL ${skill.skillName} ---`, + skill.skillDefinition.trim(), + `--- END LOCAL SKILL ${skill.skillName} ---`, + `Installed skill path: ${skill.skillRoot}`, + ); } + if (unmentioned.length > 0) { + sections.push( + "", + "Other local skills installed for this run (read a skill's SKILL.md from its path when referenced):", + ...unmentioned.map( + (skill) => `- /${skill.skillName}: ${skill.skillRoot}`, + ), + ); + } + return { context: sections.join("\n") }; + } - return { - skillName: invocation.skillName, - context: this.buildInstalledSkillPrompt(installedSkill, invocation.args), - }; + /** + * Other skills already installed for this run (auto-bundled dependencies, + * skills from earlier messages), listed so the model can find them by path. + */ + private getCoInstalledSkillBundles( + runId: string, + invokedSkillName: string, + ): InstalledSkillBundle[] { + const prefix = `${runId}:`; + return [...this.installedSkillBundleInfo.entries()] + .filter( + ([key, skill]) => + key.startsWith(prefix) && skill.skillName !== invokedSkillName, + ) + .map(([, skill]) => skill) + .sort((a, b) => a.skillName.localeCompare(b.skillName)); } private parseLocalSkillInvocation( @@ -2367,6 +2451,7 @@ export class AgentServer { private buildInstalledSkillPrompt( skill: InstalledSkillBundle, args: string | undefined, + coInstalledSkills: InstalledSkillBundle[] = [], ): string { return [ `The user invoked the local skill "/${skill.skillName}". Apply these skill instructions for this turn.`, @@ -2376,6 +2461,16 @@ export class AgentServer { `--- END LOCAL SKILL ${skill.skillName} ---`, "", `Installed skill path: ${skill.skillRoot}`, + ...(coInstalledSkills.length > 0 + ? [ + "", + "Other local skills installed for this run (when the skill above references one of these, read its SKILL.md from the listed path):", + ...coInstalledSkills.map( + (coInstalled) => + `- /${coInstalled.skillName}: ${coInstalled.skillRoot}`, + ), + ] + : []), "", "User request:", args?.trim() || `Run /${skill.skillName}.`,