diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index d42199cb..abc58d31 100644 --- a/src/CodexAcpClient.ts +++ b/src/CodexAcpClient.ts @@ -51,7 +51,7 @@ import packageJson from "../package.json"; import type {AuthenticationStatusResponse} from "./AcpExtensions"; import {createCodexCollaborationMode} from "./CollaborationModeConfig"; import type {ModeKind} from "./app-server/ModeKind"; -import {arePathBasenamesEqual, arePathsEqual, isAbsolutePathLike} from "./PathUtils"; +import {arePathBasenamesEqual, arePathsEqual, isAbsolutePathLike, normalizePathForComparison} from "./PathUtils"; /** * Well-known provider id for the client-configurable custom LLM gateway. @@ -96,6 +96,8 @@ export class CodexAcpClient { private pendingAccountUpdated: Promise | null = null; private readonly sessionNotificationQueues = new Map>(); private skillExtraRoots: string[] = []; + /** `SKILL.md` path (normalized for comparison) to skill name, from the last `listSkills` Codex answered. */ + private readonly skillNamesByPath = new Map(); private configPath: string | null = null; @@ -630,10 +632,33 @@ export class CodexAcpClient { await this.codexClient.skillsExtraRootsSet({ extraRoots: skillExtraRoots }); this.skillExtraRoots = skillExtraRoots; } - await this.codexClient.listSkills({ + const response = await this.codexClient.listSkills({ cwds: [cwd, ...additionalRoots], forceReload: true, }); + this.rememberSkillPaths(response); + } + + /** + * Index the skills Codex just reported by the path of their `SKILL.md`, so a later command that reads one can + * be recognized as a skill load and named from Codex's own registry rather than from the path's shape. + */ + private rememberSkillPaths(response: SkillsListResponse | undefined): void { + this.skillNamesByPath.clear(); + for (const entry of response?.data ?? []) { + for (const skill of entry.skills) { + this.skillNamesByPath.set(normalizePathForComparison(skill.path), skill.name); + } + } + } + + /** + * The skill whose `SKILL.md` is at [filePath], or `undefined` when that file is not a known skill's. Reading a + * skill's file is how Codex loads it, so this is what turns such a read into a skill load for clients. + */ + skillForPath(filePath: string): { name: string; path: string } | undefined { + const name = this.skillNamesByPath.get(normalizePathForComparison(filePath)); + return name === undefined ? undefined : { name, path: filePath }; } /** diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index dc21a60a..8b1e70d9 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -1599,7 +1599,7 @@ export class CodexAcpServer { case "fileChange": return [await createFileChangeUpdate(item)]; case "commandExecution": { - const updates = [await createCommandExecutionUpdate(item)]; + const updates = [await createCommandExecutionUpdate(item, path => this.codexAcpClient.skillForPath(path))]; const completeUpdate = createCommandExecutionCompleteUpdate(item, sessionState.terminalOutputMode); if (completeUpdate) { updates.push(completeUpdate); @@ -2060,6 +2060,7 @@ export class CodexAcpServer { clientSupportsPlanUpdates(this.clientCapabilities), clientSupportsTypedSessionFailures(this.clientCapabilities), this.sessionFailureEpoch, + path => this.codexAcpClient.skillForPath(path), ); eventHandler = promptEventHandler; const approvalHandler = new CodexApprovalHandler(this.connection, sessionState, activePrompt.signal); diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index df2ebf95..36b3bc8b 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -42,6 +42,7 @@ import { createCollabAgentToolCallCompleteUpdate, createCollabAgentToolCallUpdate, createCommandExecutionUpdate, + type SkillFileLookup, createContextCompactionCompleteUpdate, createContextCompactionStartUpdate, createDynamicToolCallUpdate, @@ -145,6 +146,7 @@ export class CodexEventHandler { private readonly supportsPlanUpdates: boolean; private readonly supportsTypedSessionFailures: boolean; private readonly sessionFailureEpoch: string; + private readonly skillForPath: SkillFileLookup | undefined; private readonly pendingErrors: ErrorNotification[] = []; private failure: RequestError | null = null; private completedPlan: CompletedPlan | null = null; @@ -171,11 +173,13 @@ export class CodexEventHandler { supportsPlanUpdates = false, supportsTypedSessionFailures = false, sessionFailureEpoch: string = randomUUID(), + skillForPath?: SkillFileLookup, ) { this.sessionState = sessionState; this.supportsPlanUpdates = supportsPlanUpdates; this.supportsTypedSessionFailures = supportsTypedSessionFailures; this.sessionFailureEpoch = sessionFailureEpoch; + this.skillForPath = skillForPath; this.session = new ACPSessionConnection(connection, sessionState.sessionId); } @@ -568,7 +572,7 @@ export class CodexEventHandler { this.terminalCommandIds.delete(event.item.id); this.terminalCommandOutputIds.delete(event.item.id); } - return await createCommandExecutionUpdate(event.item); + return await createCommandExecutionUpdate(event.item, this.skillForPath); } case "mcpToolCall": return await createMcpToolCallUpdate(event.item); diff --git a/src/CodexToolCallMapper.ts b/src/CodexToolCallMapper.ts index 3402cfc0..235a2689 100644 --- a/src/CodexToolCallMapper.ts +++ b/src/CodexToolCallMapper.ts @@ -45,6 +45,13 @@ type CommandExecutionItem = ThreadItem & { type: "commandExecution" }; type ContextCompactionItem = ThreadItem & { type: "contextCompaction" }; type AcpToolCallEvent = Extract; +/** + * Resolves the skill whose `SKILL.md` sits at a path, or `undefined` when that file belongs to no known skill. + * Supplied by the session that owns Codex's skill registry; absent where no registry is available, such as when + * replaying history, in which case a skill read stays an ordinary file read. + */ +export type SkillFileLookup = (filePath: string) => { name: string; path: string } | undefined; + const CONTEXT_COMPACTION_META = { contextCompaction: true }; function toAcpStatus(status: CodexItemStatus): AcpToolCallStatus { @@ -78,10 +85,13 @@ export async function createFileChangeUpdate( }; } -export async function createCommandExecutionUpdate(item: CommandExecutionItem): Promise { +export async function createCommandExecutionUpdate( + item: CommandExecutionItem, + skillForPath?: SkillFileLookup +): Promise { const commandAction = item.commandActions.length === 1 ? item.commandActions[0] : undefined; if (commandAction) { - return createCommandActionEvent(item.id, item.status, item.cwd, commandAction); + return createCommandActionEvent(item.id, item.status, item.cwd, commandAction, skillForPath); } const command = stripShellPrefix(item.command); return createTerminalCommandEvent({ @@ -540,11 +550,26 @@ export function createCommandActionEvent( id: string, status: CommandExecutionStatus, cwd: string, - commandAction: CommandAction + commandAction: CommandAction, + skillForPath?: SkillFileLookup ): AcpToolCallEvent { const acpStatus = toAcpStatus(status); switch (commandAction.type) { - case "read": + case "read": { + // Codex has no skill tool: it loads a skill by reading that skill's SKILL.md. Naming the skill here, + // from Codex's own registry, lets clients render the load as such instead of as a plain file read. + const skill = skillForPath?.(commandAction.path); + if (skill) { + return { + sessionUpdate: "tool_call", + toolCallId: id, + status: acpStatus, + kind: "read", + title: `Read skill '${skill.name}'`, + locations: [{ path: skill.path }], + _meta: { codex: { skill: skill.name, skillPath: skill.path } }, + }; + } return { sessionUpdate: "tool_call", toolCallId: id, @@ -553,6 +578,7 @@ export function createCommandActionEvent( title: `Read file '${commandAction.path}'`, locations: [{ path: commandAction.path }], }; + } case "search": return { sessionUpdate: "tool_call", diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index d1df6999..e5a3b84b 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -319,6 +319,36 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(newSessionResponse.sessionId).toBeDefined(); }); + it('should index skills by their SKILL.md path so a read of one can name the skill', async () => { + const skillFixture = createTestFixture(); + const codexAcpAgent = skillFixture.getCodexAcpAgent(); + const skillPath = '/test/project/.agents/skills/commits/SKILL.md'; + vi.spyOn(skillFixture.getCodexAppServerClient(), 'listSkills').mockResolvedValue({ + data: [{ + cwd: '/test/project', + skills: [{ + name: 'commits', + description: 'Write commit messages', + path: skillPath, + scope: 'repo', + enabled: true, + }], + errors: [], + }], + }); + + await codexAcpAgent.initialize({protocolVersion: 1}); + await codexAcpAgent.authenticate({ + methodId: "api-key", + _meta: { "api-key": { apiKey: "TOKEN" } }, + } satisfies CodexAuthRequest); + await codexAcpAgent.newSession({cwd: '/test/project', mcpServers: []}); + + // Codex loads a skill by reading its SKILL.md, so the registry is what turns that read into a named load. + expect(skillFixture.getCodexAcpClient().skillForPath(skillPath)).toEqual({name: 'commits', path: skillPath}); + expect(skillFixture.getCodexAcpClient().skillForPath('/test/project/docs/SKILL.md')).toBeUndefined(); + }); + it('should show account in /status for api key auth and hide it for gateway auth', async () => { const authFixture = createTestFixture(); const codexAcpAgent = authFixture.getCodexAcpAgent(); diff --git a/src/__tests__/CodexACPAgent/command-action-events.test.ts b/src/__tests__/CodexACPAgent/command-action-events.test.ts index 658a9976..0c01f815 100644 --- a/src/__tests__/CodexACPAgent/command-action-events.test.ts +++ b/src/__tests__/CodexACPAgent/command-action-events.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { createCommandActionEvent } from '../../CodexToolCallMapper'; import type { SessionState } from '../../CodexAcpServer'; import type { ServerNotification } from '../../app-server'; import { createCodexMockTestFixture, createTestSessionState, setupPromptAndSendNotifications, type CodexMockTestFixture } from '../acp-test-utils'; @@ -490,4 +491,58 @@ describe('CodexEventHandler - command action events', () => { 'data/dynamic-tool-in-progress.json' ); }); + + describe('skill loads', () => { + // Codex has no skill tool: it loads a skill by reading that skill's SKILL.md, so the read of a path the + // skill registry knows is what identifies the load. + const skillPath = '/test/project/.agents/skills/commits/SKILL.md'; + const readSkill = (path: string) => ({ + type: 'read' as const, + command: `cat ${path}`, + name: 'cat', + path, + }); + + it('names the skill and carries its path when the read is a known skill file', () => { + const event = createCommandActionEvent( + 'command-read-skill', + 'inProgress', + '/test/project', + readSkill(skillPath), + path => (path === skillPath ? { name: 'commits', path } : undefined) + ); + + expect(event).toMatchObject({ + kind: 'read', + title: "Read skill 'commits'", + locations: [{ path: skillPath }], + _meta: { codex: { skill: 'commits', skillPath } }, + }); + }); + + it('leaves a read of an unknown file as an ordinary file read', () => { + const event = createCommandActionEvent( + 'command-read-file', + 'inProgress', + '/test/project', + readSkill('/test/project/docs/SKILL.md'), + path => (path === skillPath ? { name: 'commits', path } : undefined) + ); + + expect(event.title).toBe("Read file '/test/project/docs/SKILL.md'"); + expect(event._meta).toBeUndefined(); + }); + + it('leaves reads alone when no skill registry is available', () => { + const event = createCommandActionEvent( + 'command-read-no-registry', + 'inProgress', + '/test/project', + readSkill(skillPath) + ); + + expect(event.title).toBe(`Read file '${skillPath}'`); + expect(event._meta).toBeUndefined(); + }); + }); });