Skip to content

Commit 3a3e8f6

Browse files
authored
Fix skill listing for session cwd (#251)
Use the active ACP session roots when publishing skill commands and handling /skills so Codex app-server does not fall back to the adapter process cwd. Adds a portable e2e regression for repo-local skills under the session workspace. Fixes #250
1 parent 211b7d7 commit 3a3e8f6

5 files changed

Lines changed: 68 additions & 12 deletions

File tree

src/CodexAcpServer.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ export class CodexAcpServer {
414414
this.publishMcpStartupStatusAsync(sessionId);
415415
}
416416

417-
this.publishAvailableCommandsAsync(sessionId);
417+
this.publishAvailableCommandsAsync(sessionState);
418418
const sessionModelState: LegacySessionModelState = this.createModelState(models, currentModelId);
419419
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
420420

@@ -804,8 +804,8 @@ export class CodexAcpServer {
804804
return !isJetBrains2026_1Client(this.clientInfo);
805805
}
806806

807-
private publishAvailableCommandsAsync(sessionId: string) {
808-
void this.availableCommands.publish(sessionId);
807+
private publishAvailableCommandsAsync(sessionState: SessionState) {
808+
void this.availableCommands.publish(sessionState);
809809
}
810810

811811
private findCurrentModel(models: Model[], currentModelId: string): Model | undefined {
@@ -917,7 +917,7 @@ export class CodexAcpServer {
917917
this.publishMcpStartupStatusAsync(sessionId);
918918
}
919919

920-
await this.availableCommands.publish(sessionId);
920+
await this.availableCommands.publish(sessionState);
921921
const sessionModelState: LegacySessionModelState = this.createModelState(models, currentModelId);
922922
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
923923

src/CodexCommands.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type * as acp from "@agentclientprotocol/sdk";
22
import type {AvailableCommand} from "@agentclientprotocol/sdk";
33
import {ACPSessionConnection, type AcpClientConnection} from "./ACPSessionConnection";
44
import type {CodexAcpClient} from "./CodexAcpClient";
5-
import type {RateLimitSnapshot, ReviewTarget, SkillsListEntry, TurnCompletedNotification} from "./app-server/v2";
5+
import type {RateLimitSnapshot, ReviewTarget, SkillsListEntry, SkillsListParams, TurnCompletedNotification} from "./app-server/v2";
66
import type {SessionState} from "./CodexAcpServer";
77
import type {RateLimitsMap} from "./RateLimitsMap";
88
import type {TokenCount} from "./TokenCount";
@@ -42,24 +42,30 @@ export class CodexCommands {
4242
this.onLogout = onLogout;
4343
}
4444

45-
async publish(sessionId: string): Promise<void> {
45+
async publish(sessionState: SessionState): Promise<void> {
4646
try {
47-
const skillsResponse = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills());
47+
const skillsResponse = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills(this.createSkillsListParams(sessionState)));
4848
const availableCommands = this.buildAvailableCommands(skillsResponse?.data ?? []);
4949
if (availableCommands.length === 0) {
5050
return;
5151
}
5252

53-
const session = new ACPSessionConnection(this.connection, sessionId);
53+
const session = new ACPSessionConnection(this.connection, sessionState.sessionId);
5454
await session.update({
5555
sessionUpdate: "available_commands_update",
5656
availableCommands
5757
});
5858
} catch (err) {
59-
logger.error(`Failed to publish available commands for session ${sessionId}`, err);
59+
logger.error(`Failed to publish available commands for session ${sessionState.sessionId}`, err);
6060
}
6161
}
6262

63+
private createSkillsListParams(sessionState: SessionState): SkillsListParams {
64+
return {
65+
cwds: [sessionState.cwd, ...sessionState.additionalDirectories],
66+
};
67+
}
68+
6369
private buildAvailableCommands(skillsEntries: SkillsListEntry[]): AvailableCommand[] {
6470
const commands = new Map<string, AvailableCommand>();
6571

@@ -221,7 +227,7 @@ export class CodexCommands {
221227
return { handled: true };
222228
}
223229
case "skills": {
224-
const response = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills());
230+
const response = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills(this.createSkillsListParams(sessionState)));
225231
const skills = (response?.data ?? []).flatMap(entry => entry.skills);
226232
const lines = skills.map(skill => {
227233
const description = skill.shortDescription ?? skill.description ?? "";

src/__tests__/CodexACPAgent/CodexAcpClient.test.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,7 +1250,14 @@ describe('ACP server test', { timeout: 40_000 }, () => {
12501250
vi.spyOn(mockFixture.getCodexAcpClient(), "listSkills").mockResolvedValue({ data: [] });
12511251

12521252
// @ts-expect-error - exercising private helper
1253-
await codexAcpAgent.availableCommands.publish("session-id");
1253+
await codexAcpAgent.availableCommands.publish(createTestSessionState({
1254+
sessionId: "session-id",
1255+
cwd: "/workspace",
1256+
}));
1257+
1258+
expect(mockFixture.getCodexAcpClient().listSkills).toHaveBeenCalledWith({
1259+
cwds: ["/workspace"],
1260+
});
12541261

12551262
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/available-commands-build-in.json");
12561263
});
@@ -1275,7 +1282,15 @@ describe('ACP server test', { timeout: 40_000 }, () => {
12751282
});
12761283

12771284
// @ts-expect-error - exercising private helper
1278-
await codexAcpAgent.availableCommands.publish("session-id");
1285+
await codexAcpAgent.availableCommands.publish(createTestSessionState({
1286+
sessionId: "session-id",
1287+
cwd: "/workspace",
1288+
additionalDirectories: ["/workspace/extra"],
1289+
}));
1290+
1291+
expect(mockFixture.getCodexAcpClient().listSkills).toHaveBeenCalledWith({
1292+
cwds: ["/workspace", "/workspace/extra"],
1293+
});
12791294

12801295
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/available-commands-skills.json");
12811296
});

src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,21 @@ describeE2E("E2E tests", () => {
128128
expect(text).toContain("- session-root-skill: Session root skill");
129129
});
130130
});
131+
132+
it("lists repo skills from the session cwd", async () => {
133+
fixture = await createAuthenticatedFixture();
134+
fixture.writeSkill({
135+
name: "workspace-skill",
136+
description: "Workspace skill",
137+
body: "This skill exists only in the session workspace.",
138+
}, path.join(fixture.workspaceDir, ".agents", "skills"));
139+
140+
const session = await fixture.createSession();
141+
142+
await fixture.expectAvailableCommand(session.sessionId, "$workspace-skill");
143+
await fixture.expectPromptText(session.sessionId, "/skills", (text) => {
144+
expect(text).toContain("Available skills:");
145+
expect(text).toContain("- workspace-skill: Workspace skill");
146+
});
147+
});
131148
});

src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export interface SpawnedAgentFixture {
2525
restart(): Promise<SpawnedAgentFixture>;
2626
writeSkill(skill: TestSkill, rootDir?: string): void;
2727
setPermissionResponder(responder: PermissionResponder): void;
28+
expectAvailableCommand(sessionId: string, commandName: string, timeoutMs?: number): Promise<void>;
2829
expectPromptText(
2930
sessionId: string,
3031
promptText: string,
@@ -103,6 +104,7 @@ class RuntimePaths {
103104

104105
class RecordingClient implements acp.Client {
105106
private readonly textBySessionId = new Map<string, string>();
107+
private readonly availableCommandsBySessionId = new Map<string, acp.AvailableCommand[]>();
106108
private readonly permissionRequestsBySessionId = new Map<string, acp.RequestPermissionRequest[]>();
107109
private permissionResponder: PermissionResponder = () => ({
108110
outcome: {outcome: "cancelled"},
@@ -123,6 +125,11 @@ class RecordingClient implements acp.Client {
123125
}
124126

125127
async sessionUpdate(params: acp.SessionNotification): Promise<void> {
128+
if (params.update.sessionUpdate === "available_commands_update") {
129+
this.availableCommandsBySessionId.set(params.sessionId, params.update.availableCommands);
130+
return;
131+
}
132+
126133
if (params.update.sessionUpdate !== "agent_message_chunk" || params.update.content.type !== "text") {
127134
return;
128135
}
@@ -135,6 +142,10 @@ class RecordingClient implements acp.Client {
135142
return this.textBySessionId.get(sessionId) ?? "";
136143
}
137144

145+
readAvailableCommands(sessionId: string): acp.AvailableCommand[] {
146+
return this.availableCommandsBySessionId.get(sessionId) ?? [];
147+
}
148+
138149
readPermissionRequests(
139150
sessionId: string,
140151
toolCallKind: acp.ToolKind,
@@ -204,6 +215,13 @@ class SpawnedAgentFixtureImpl implements SpawnedAgentFixture {
204215
this.client.setPermissionResponder(responder);
205216
}
206217

218+
async expectAvailableCommand(sessionId: string, commandName: string, timeoutMs = 30_000): Promise<void> {
219+
await vi.waitFor(() => {
220+
const commandNames = this.client.readAvailableCommands(sessionId).map(command => command.name);
221+
expect(commandNames).toContain(commandName);
222+
}, {timeout: timeoutMs});
223+
}
224+
207225
async expectPromptText(
208226
sessionId: string,
209227
promptText: string,

0 commit comments

Comments
 (0)