Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 27 additions & 2 deletions src/CodexAcpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -96,6 +96,8 @@ export class CodexAcpClient {
private pendingAccountUpdated: Promise<AccountUpdatedNotification> | null = null;
private readonly sessionNotificationQueues = new Map<string, Promise<void>>();
private skillExtraRoots: string[] = [];
/** `SKILL.md` path (normalized for comparison) to skill name, from the last `listSkills` Codex answered. */
private readonly skillNamesByPath = new Map<string, string>();
private configPath: string | null = null;


Expand Down Expand Up @@ -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 };
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/CodexAcpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion src/CodexEventHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
createCollabAgentToolCallCompleteUpdate,
createCollabAgentToolCallUpdate,
createCommandExecutionUpdate,
type SkillFileLookup,
createContextCompactionCompleteUpdate,
createContextCompactionStartUpdate,
createDynamicToolCallUpdate,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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);
Expand Down
34 changes: 30 additions & 4 deletions src/CodexToolCallMapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ type CommandExecutionItem = ThreadItem & { type: "commandExecution" };
type ContextCompactionItem = ThreadItem & { type: "contextCompaction" };
type AcpToolCallEvent = Extract<UpdateSessionEvent, { sessionUpdate: "tool_call" }>;

/**
* 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 {
Expand Down Expand Up @@ -78,10 +85,13 @@ export async function createFileChangeUpdate(
};
}

export async function createCommandExecutionUpdate(item: CommandExecutionItem): Promise<UpdateSessionEvent> {
export async function createCommandExecutionUpdate(
item: CommandExecutionItem,
skillForPath?: SkillFileLookup
): Promise<UpdateSessionEvent> {
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({
Expand Down Expand Up @@ -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,
Expand All @@ -553,6 +578,7 @@ export function createCommandActionEvent(
title: `Read file '${commandAction.path}'`,
locations: [{ path: commandAction.path }],
};
}
case "search":
return {
sessionUpdate: "tool_call",
Expand Down
30 changes: 30 additions & 0 deletions src/__tests__/CodexACPAgent/CodexAcpClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
55 changes: 55 additions & 0 deletions src/__tests__/CodexACPAgent/command-action-events.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
});
});
});