Skip to content

Commit 06a749b

Browse files
AlexandrSuhininAlexandrSuhinin
authored
fix commands with attachments (#147)
* clean: remove unused abstraction * fix: handle commands with attachments
1 parent 64d3bc2 commit 06a749b

6 files changed

Lines changed: 81 additions & 70 deletions

File tree

‎src/CodexAcpServer.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -790,7 +790,7 @@ export class CodexAcpServer implements acp.Agent {
790790
approvalHandler,
791791
elicitationHandler);
792792

793-
if (await this.availableCommands.tryHandle(params.prompt, sessionState)) {
793+
if (await this.availableCommands.tryHandleCommand(params.prompt, sessionState)) {
794794
logger.log("Prompt handled by a command");
795795
return {
796796
stopReason: "end_turn",

‎src/CodexCommands.ts‎

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,6 @@ export class CodexCommands {
4141
}
4242
}
4343

44-
async tryHandle(prompt: acp.ContentBlock[], sessionState: SessionState): Promise<boolean> {
45-
const command = this.parseCommand(prompt);
46-
if (command) {
47-
return this.handleCommand(command, sessionState);
48-
}
49-
return false;
50-
}
51-
5244
private buildAvailableCommands(skillsEntries: SkillsListEntry[]): AvailableCommand[] {
5345
const commands = new Map<string, AvailableCommand>();
5446

@@ -99,30 +91,26 @@ export class CodexCommands {
9991
];
10092
}
10193

102-
private parseCommand(prompt: acp.ContentBlock[]): ParsedCommand | null {
103-
if (prompt.length !== 1) return null;
104-
const [single] = prompt;
105-
if (!single) return null;
94+
private getCommandName(prompt: acp.ContentBlock[]): string | null {
95+
const firstBlock = prompt[0];
96+
if (!firstBlock || firstBlock.type != "text") return null;
10697

107-
if (single.type !== "text") return null;
108-
const trimmed = single.text.trim();
109-
if (!trimmed.startsWith("/")) return null;
98+
const text = firstBlock.text.trim();
99+
if (!text.startsWith("/")) return null;
110100

111-
const commandText = trimmed.slice(1).trim();
101+
const commandText = text.slice(1).trim();
112102
if (commandText.length === 0) return null;
113103

114-
const [name, ...rest] = commandText.split(/\s+/);
115-
const input = rest.join(" ").trim();
116-
return {
117-
name: name!!.toLowerCase(),
118-
input: input.length > 0 ? input : null
119-
};
104+
const [name] = commandText.split(/\s+/);
105+
return name?.toLowerCase() ?? null;
120106
}
121107

122-
async handleCommand(command: ParsedCommand, sessionState: SessionState): Promise<boolean> {
123-
const sessionId = sessionState.sessionId;
108+
async tryHandleCommand(prompt: acp.ContentBlock[], sessionState: SessionState): Promise<boolean> {
109+
const commandName = this.getCommandName(prompt)
110+
if (commandName === null) return false;
124111

125-
switch (command.name) {
112+
const sessionId = sessionState.sessionId;
113+
switch (commandName) {
126114
case "status": {
127115
const session = new ACPSessionConnection(this.connection, sessionId);
128116
const message = this.buildStatusMessage(sessionState);
@@ -180,7 +168,7 @@ export class CodexCommands {
180168
return true;
181169
}
182170
default:
183-
await this.sendUnknownCommandMessage(command.name, sessionId);
171+
await this.sendUnknownCommandMessage(commandName, sessionId);
184172
return true;
185173
}
186174
}
@@ -354,4 +342,4 @@ export class CodexCommands {
354342
}
355343
}
356344

357-
type ParsedCommand = { name: string; input: string | null };
345+
type ParsedCommand = { name: string; };

‎src/__tests__/CodexACPAgent/CodexAcpClient.test.ts‎

Lines changed: 62 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11
// noinspection ES6RedundantAwait
22

3-
import {describe, expect, it, vi, beforeEach} from 'vitest';
3+
import {beforeEach, describe, expect, it, vi} from 'vitest';
44
import type {CodexAuthRequest} from "../../CodexAuthMethod";
55
import type * as acp from "@agentclientprotocol/sdk";
6-
import {createTestFixture, createCodexMockTestFixture, createTestSessionState, type TestFixture} from "../acp-test-utils";
6+
import {
7+
createCodexMockTestFixture,
8+
createTestFixture,
9+
createTestSessionState,
10+
type TestFixture
11+
} from "../acp-test-utils";
712
import type {ServerNotification} from "../../app-server";
813
import type {SessionState} from "../../CodexAcpServer";
914
import {AgentMode} from "../../AgentMode";
10-
import type {ListMcpServerStatusResponse, Model, SkillsListResponse, TurnStartParams} from "../../app-server/v2";
15+
import type {Model, TurnStartParams} from "../../app-server/v2";
1116
import type {RateLimitsMap} from "../../RateLimitsMap";
1217
import {ModelId} from "../../ModelId";
1318

@@ -736,28 +741,25 @@ describe('ACP server test', { timeout: 40_000 }, () => {
736741
});
737742

738743
it('handles logout command', async () => {
739-
const mockFixture = createCodexMockTestFixture();
740-
const codexAcpAgent = mockFixture.getCodexAcpAgent();
741-
742-
const sessionState: SessionState = createTestSessionState();
744+
const codexAcpAgent = fixture.getCodexAcpAgent();
745+
await codexAcpAgent.initialize({protocolVersion: 1});
743746

744-
const logoutSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "logout").mockResolvedValue();
747+
fixture.getCodexAcpClient().authRequired = vi.fn().mockResolvedValue(false);
745748

746-
// @ts-expect-error - exercising private helper
747-
const handled = await codexAcpAgent.availableCommands.handleCommand({ name: "logout", input: null }, sessionState);
749+
const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
748750

749-
expect(handled).toBe(true);
750-
expect(logoutSpy).toHaveBeenCalledTimes(1);
751-
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/command-logout.json");
751+
fixture.clearAcpConnectionDump();
752+
const prompt: acp.ContentBlock[] = [{ type: "text", text: "/logout " }];
753+
await codexAcpAgent.prompt({sessionId: newSessionResponse.sessionId, prompt: prompt });
754+
await expect(fixture.getAcpConnectionDump(["sessionId"])).toMatchFileSnapshot("data/command-logout.json");
752755
});
753756

754757
it('handles skills command', async () => {
755-
const mockFixture = createCodexMockTestFixture();
756-
const codexAcpAgent = mockFixture.getCodexAcpAgent();
757-
758-
const sessionState: SessionState = createTestSessionState();
758+
const codexAcpAgent = fixture.getCodexAcpAgent();
759+
await codexAcpAgent.initialize({protocolVersion: 1});
759760

760-
const skillsResponse: SkillsListResponse = {
761+
fixture.getCodexAcpClient().authRequired = vi.fn().mockResolvedValue(false);
762+
vi.spyOn(fixture.getCodexAcpClient(), "listSkills").mockResolvedValue({
761763
data: [{
762764
cwd: "/workspace",
763765
skills: [
@@ -766,29 +768,28 @@ describe('ACP server test', { timeout: 40_000 }, () => {
766768
],
767769
errors: []
768770
}]
769-
};
770-
const skillsSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "listSkills").mockResolvedValue(skillsResponse);
771+
});
771772

772-
// @ts-expect-error - exercising private helper
773-
const handled = await codexAcpAgent.availableCommands.handleCommand({ name: "skills", input: null }, sessionState);
773+
const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
774774

775-
expect(handled).toBe(true);
776-
expect(skillsSpy).toHaveBeenCalledTimes(1);
777-
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/command-skills.json");
775+
fixture.clearAcpConnectionDump();
776+
const prompt: acp.ContentBlock[] = [{ type: "text", text: "/skills " }];
777+
await codexAcpAgent.prompt({sessionId: newSessionResponse.sessionId, prompt: prompt });
778+
779+
await expect(fixture.getAcpConnectionDump(["sessionId"])).toMatchFileSnapshot("data/command-skills.json");
778780
});
779781

780782
it('handles mcp command', async () => {
781-
const mockFixture = createCodexMockTestFixture();
782-
const codexAcpAgent = mockFixture.getCodexAcpAgent();
783-
784-
const sessionState: SessionState = createTestSessionState();
783+
const codexAcpAgent = fixture.getCodexAcpAgent();
784+
await codexAcpAgent.initialize({protocolVersion: 1});
785785

786-
const mcpResponse: ListMcpServerStatusResponse = {
786+
fixture.getCodexAcpClient().authRequired = vi.fn().mockResolvedValue(false);
787+
vi.spyOn(fixture.getCodexAcpClient(), "listMcpServers").mockResolvedValue({
787788
data: [
788789
{
789790
name: "fs",
790-
tools: { listFiles: { name: "listFiles", inputSchema: { type: "object" } } },
791-
resources: [{ name: "workspace", uri: "file:///workspace" }],
791+
tools: {listFiles: {name: "listFiles", inputSchema: {type: "object"}}},
792+
resources: [{name: "workspace", uri: "file:///workspace"}],
792793
resourceTemplates: [],
793794
authStatus: "bearerToken"
794795
},
@@ -801,15 +802,37 @@ describe('ACP server test', { timeout: 40_000 }, () => {
801802
}
802803
],
803804
nextCursor: null
804-
};
805-
const mcpSpy = vi.spyOn(mockFixture.getCodexAcpClient(), "listMcpServers").mockResolvedValue(mcpResponse);
805+
});
806806

807-
// @ts-expect-error - exercising private helper
808-
const handled = await codexAcpAgent.availableCommands.handleCommand({ name: "mcp", input: null }, sessionState);
807+
const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
808+
809+
fixture.clearAcpConnectionDump();
810+
const prompt: acp.ContentBlock[] = [{ type: "text", text: "/mcp " }];
811+
await codexAcpAgent.prompt({sessionId: newSessionResponse.sessionId, prompt: prompt });
812+
await expect(fixture.getAcpConnectionDump(["sessionId"])).toMatchFileSnapshot("data/command-mcp.json");
813+
});
814+
815+
it('handles builtin slash command locally when prompt has attachments', async () => {
816+
const codexAcpAgent = fixture.getCodexAcpAgent();
817+
await codexAcpAgent.initialize({protocolVersion: 1});
818+
819+
fixture.getCodexAcpClient().authRequired = vi.fn().mockResolvedValue(false);
820+
821+
const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
822+
const prompt: acp.ContentBlock[] = [
823+
{ type: "text", text: "/status " },
824+
{
825+
type: "resource_link",
826+
name: "editor.xml",
827+
uri: "file:///editor.xml",
828+
description: "File that is opened in the IDE and is currently viewed by the user",
829+
},
830+
];
809831

810-
expect(handled).toBe(true);
811-
expect(mcpSpy).toHaveBeenCalledTimes(1);
812-
await expect(mockFixture.getAcpConnectionDump([])).toMatchFileSnapshot("data/command-mcp.json");
832+
fixture.clearAcpConnectionDump();
833+
await codexAcpAgent.prompt({sessionId: newSessionResponse.sessionId, prompt: prompt });
834+
const transportDump = fixture.getAcpConnectionDump([]);
835+
expect(transportDump).contain(`**Session:** \`${newSessionResponse.sessionId}\``);
813836
});
814837

815838
const mockModels: Model[] = [

‎src/__tests__/CodexACPAgent/data/command-logout.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"method": "sessionUpdate",
33
"args": [
44
{
5-
"sessionId": "session-id",
5+
"sessionId": "sessionId",
66
"update": {
77
"sessionUpdate": "agent_message_chunk",
88
"content": {

‎src/__tests__/CodexACPAgent/data/command-mcp.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"method": "sessionUpdate",
33
"args": [
44
{
5-
"sessionId": "session-id",
5+
"sessionId": "sessionId",
66
"update": {
77
"sessionUpdate": "agent_message_chunk",
88
"content": {

‎src/__tests__/CodexACPAgent/data/command-skills.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"method": "sessionUpdate",
33
"args": [
44
{
5-
"sessionId": "session-id",
5+
"sessionId": "sessionId",
66
"update": {
77
"sessionUpdate": "agent_message_chunk",
88
"content": {

0 commit comments

Comments
 (0)