Don't init codex client instead mock it in test & refactoring

Author: Aleksandr Slapoguzov

src/CodexAcpClient.ts

@@ -14,7 +14,6 @@ import type {
 import type {JsonValue} from "./app-server/serde_json/JsonValue";
 import {ModelId} from "./ModelId";
 import {AgentMode} from "./AgentMode";
-import {CodexAdditionalRootsProvider} from "./CodexAdditionalRootsProvider";
 import path from "node:path";
 import {logger} from "./Logger";
 import type {
@@ -39,15 +38,13 @@ import type {AuthenticationLogoutResponse, AuthenticationStatusResponse} from ".
 export class CodexAcpClient {
 
     private readonly codexClient: CodexAppServerClient;
-    private readonly additionalRootsProvider: CodexAdditionalRootsProvider;
     private readonly config: JsonObject;
     private readonly modelProvider: string | null;
     private gatewayConfig: GatewayConfig | null;
 
 
     constructor(codexClient: CodexAppServerClient, codexConfig?: JsonObject, modelProvider?: string) {
         this.codexClient = codexClient;
-        this.additionalRootsProvider = new CodexAdditionalRootsProvider(codexClient);
         this.config = codexConfig ?? {};
         this.modelProvider = modelProvider ?? null;
         this.gatewayConfig = null;
@@ -182,7 +179,7 @@ export class CodexAcpClient {
     }
 
     async resumeSession(request: acp.ResumeSessionRequest): Promise<SessionMetadata> {
-        await this.additionalRootsProvider.refreshSkills(request);
+        await this.refreshSkills(request.cwd, request._meta);
 
         const response = await this.codexClient.threadResume({
             approvalPolicy: null,
@@ -233,7 +230,7 @@ export class CodexAcpClient {
     }
 
     async newSession(request: acp.NewSessionRequest): Promise<SessionMetadata> {
-        await this.additionalRootsProvider.refreshSkills(request);
+        await this.refreshSkills(request.cwd, request._meta);
 
         const response = await this.codexClient.threadStart({
             config: this.createSessionConfig(request.cwd, request.mcpServers),
@@ -288,6 +285,21 @@ export class CodexAcpClient {
         return this.gatewayConfig?.modelProvider ?? this.modelProvider;
     }
 
+    private async refreshSkills(cwd: string, meta?: Record<string, unknown> | null): Promise<void> {
+        if (!cwd) {
+            return;
+        }
+        const additionalRoots = readAdditionalRoots(meta);
+        await this.codexClient.listSkills({
+            cwds: [cwd],
+            forceReload: true,
+            perCwdExtraUserRoots: [{
+                cwd: cwd,
+                extraUserRoots: additionalRoots
+            }]
+        });
+    }
+
     /**
      * Create a codex config entry for MCP server
      */
@@ -343,11 +355,7 @@ export class CodexAcpClient {
         const input = buildPromptItems(request.prompt);
         const effort = modelId.effort as ReasoningEffort | null; //TODO remove unsafe conversion
 
-        const refreshSkillsRequest: { _meta?: Record<string, unknown> | null, cwd?: string } = { cwd: cwd };
-        if (request._meta !== undefined) {
-            refreshSkillsRequest._meta = request._meta;
-        }
-        await this.additionalRootsProvider.refreshSkills(refreshSkillsRequest);
+        await this.refreshSkills(cwd, request._meta);
         await this.codexClient.turnStart({
             outputSchema: null,
             threadId: request.sessionId,
@@ -572,6 +580,18 @@ interface GatewayConfig {
     }
 }
 
+function readAdditionalRoots(meta: Record<string, unknown> | null | undefined): string[] {
+    const rawRoots = meta?.["additionalRoots"];
+    if (!Array.isArray(rawRoots)) {
+        return [];
+    }
+
+    return Array.from(new Set(rawRoots
+        .filter((value): value is string => typeof value === "string")
+        .map(value => value.trim())
+        .filter(value => value.length > 0)));
+}
+
 function mergeGatewayConfig(config: JsonObject, gatewayConfig: GatewayConfig | null): JsonObject {
     if (gatewayConfig !== null) {
         const newConfig = {...config};

src/CodexAdditionalRootsProvider.ts

@@ -66,10 +66,7 @@ describe('ACP server test', { timeout: 40_000 }, () => {
         const newSessionResponse = await codexAcpAgent.newSession({cwd: "", mcpServers: []});
         expect(newSessionResponse.sessionId).toBeDefined()
 
-        const transportDump = fixture.getCodexConnectionDump(
-            [...ignoredFields, "upgrade"],
-            { ignoreNotificationMethods: ["codex/event/mcp_startup_update"] }
-        );
+        const transportDump = fixture.getCodexConnectionDump([...ignoredFields, "upgrade"]);
         await expect(transportDump).toMatchFileSnapshot("data/auth-with-key.json");
 
         const authenticatedResponse = await fixture.getCodexAcpAgent().extMethod("authentication/status", {});
@@ -216,8 +213,8 @@ describe('ACP server test', { timeout: 40_000 }, () => {
         fixture.getCodexAppServerClient().onServerNotification = loadNotifications();
 
         const codexAcpAgent = fixture.getCodexAcpAgent();
-        await codexAcpAgent.initialize({ protocolVersion: 1 });
 
+        fixture.getCodexAppServerClient().listSkills = vi.fn().mockResolvedValue({ data: [] });
         fixture.getCodexAppServerClient().turnStart = vi.fn().mockResolvedValue({
             turn: { id: "turn-id", items: [], status: "inProgress", error: null }
         });

src/__tests__/CodexACPAgent/CodexAcpClient.test.ts

@@ -26,8 +26,8 @@ describe('MCP session configuration', { timeout: 40_000 }, () => {
         fixture.clearAcpConnectionDump();
         await codexAcpAgent.prompt({sessionId: newSessionResponse.sessionId, prompt: [{type: "text", text: "/mcp"}]});
         const transportDump = fixture.getAcpConnectionDump([]);
-        expect(transportDump).toContain("Configured MCP servers:");
-        expect(transportDump).toContain("- test-mcp");
+        console.log(transportDump)
+        expect(transportDump).contain("Configured MCP servers:\\n- test-mcp");
     });
 
 });