Skip to content

Commit 3cce5e2

Browse files
committed
Fix skill listing for session cwd
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 956f71a commit 3cce5e2

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
@@ -406,7 +406,7 @@ export class CodexAcpServer {
406406
this.publishMcpStartupStatusAsync(sessionId);
407407
}
408408

409-
this.publishAvailableCommandsAsync(sessionId);
409+
this.publishAvailableCommandsAsync(sessionState);
410410
const sessionModelState: LegacySessionModelState = this.createModelState(models, currentModelId);
411411
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
412412

@@ -796,8 +796,8 @@ export class CodexAcpServer {
796796
return !isJetBrains2026_1Client(this.clientInfo);
797797
}
798798

799-
private publishAvailableCommandsAsync(sessionId: string) {
800-
void this.availableCommands.publish(sessionId);
799+
private publishAvailableCommandsAsync(sessionState: SessionState) {
800+
void this.availableCommands.publish(sessionState);
801801
}
802802

803803
private findCurrentModel(models: Model[], currentModelId: string): Model | undefined {
@@ -909,7 +909,7 @@ export class CodexAcpServer {
909909
this.publishMcpStartupStatusAsync(sessionId);
910910
}
911911

912-
await this.availableCommands.publish(sessionId);
912+
await this.availableCommands.publish(sessionState);
913913
const sessionModelState: LegacySessionModelState = this.createModelState(models, currentModelId);
914914
const sessionModeState: SessionModeState = sessionState.agentMode.toSessionModeState();
915915

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";
@@ -41,24 +41,30 @@ export class CodexCommands {
4141
this.onLogout = onLogout;
4242
}
4343

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

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

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

@@ -212,7 +218,7 @@ export class CodexCommands {
212218
return { handled: true };
213219
}
214220
case "skills": {
215-
const response = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills());
221+
const response = await this.runWithProcessCheck(() => this.codexAcpClient.listSkills(this.createSkillsListParams(sessionState)));
216222
const skills = (response?.data ?? []).flatMap(entry => entry.skills);
217223
const lines = skills.map(skill => {
218224
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)