Fix broken skills (additionalDirectories) in 0.130.0

package.json

@@ -65,6 +65,7 @@
     "@openai/codex": "^0.128.0",
     "diff": "^8.0.3",
     "open": "^11.0.0",
-    "vscode-jsonrpc": "^8.2.1"
+    "vscode-jsonrpc": "^8.2.1",
+    "yaml": "^2.9.0"
   }
 }

src/AcpExtensions.ts

@@ -1,11 +1,14 @@
-export type ExtMethodRequest = AuthenticationStatusRequest | AuthenticationLogoutRequest
+export type ExtMethodRequest =
+    | AuthenticationStatusRequest
+    | AuthenticationLogoutRequest
 
 export function isExtMethodRequest(request: { method: string, params: Record<string, unknown> }): request is ExtMethodRequest {
-    return request.method === "authentication/status" || request.method === "authentication/logout";
+    return request.method === "authentication/status"
+        || request.method === "authentication/logout"
 }
 
 export type AuthenticationStatusRequest = { method: "authentication/status", params: {} }
 export type AuthenticationStatusResponse = { type: "api-key" } | { type: "chat-gpt", email: string } | { type: "gateway", name: string } | { type: "unauthenticated" }
 
 export type AuthenticationLogoutRequest = { method: "authentication/logout", params: {} }
-export type AuthenticationLogoutResponse = {}
+export type AuthenticationLogoutResponse = {}

src/CodexAcpClient.ts

@@ -20,6 +20,7 @@ import type {JsonValue} from "./app-server/serde_json/JsonValue";
 import {ModelId} from "./ModelId";
 import {AgentMode} from "./AgentMode";
 import path from "node:path";
+import {fileURLToPath} from "node:url";
 import {logger} from "./Logger";
 import type {
     AccountLoginCompletedNotification,
@@ -36,6 +37,16 @@ import type {
 } from "./app-server/v2";
 import packageJson from "../package.json";
 import type {AuthenticationStatusResponse} from "./AcpExtensions";
+import {CODEX_SKILL_FILE_NAME} from "./SkillDirectoryParser";
+import {installAdditionalRootSkillMarketplaces} from "./LocalSkillMarketplace";
+
+const FILE_URI_PREFIX = "file://";
+// From the Codex Rust implementation, `codex-rs/core-skills/src/loader.rs` defines `SKILLS_FILENAME = "SKILL.md"` and only parses files whose discovered filename exactly matches that value. The app-server README and bundled skill-creator sample also document `SKILL.md` as the required file for a skill. There is
+// some UI/mention handling that recognizes paths ending in `SKILL.md`, but the actual loader discovery path is hardcoded around this filename.
+
+type SessionId = string;
+type AdditionalRootPath = string;
+type AdditionalRootPaths = AdditionalRootPath[];
 
 /**
  * API for accessing the Codex App Server using ACP requests.
@@ -46,6 +57,7 @@ export class CodexAcpClient {
     private readonly config: JsonObject;
     private readonly modelProvider: string | null;
     private gatewayConfig: GatewayConfig | null;
+    private readonly additionalRootPathsBySessionId = new Map<SessionId, AdditionalRootPaths>();
     private pendingLoginCompleted: Promise<AccountLoginCompletedNotification> | null = null;
     private pendingAccountUpdated: Promise<AccountUpdatedNotification> | null = null;
 
@@ -182,6 +194,17 @@ export class CodexAcpClient {
         await accountUpdatedPromise;
     }
 
+    async removeMarketplace(marketplaceName: string): Promise<void> {
+        await this.codexClient.marketplaceRemove({marketplaceName});
+    }
+
+    async listMarketplaces(cwd: string): Promise<string[]> {
+        const pluginList = await this.codexClient.pluginList({
+            cwds: cwd ? [cwd] : []
+        });
+        return pluginList.marketplaces.map((marketplace) => marketplace.name);
+    }
+
     async authRequired(): Promise<Boolean> {
         if (this.gatewayConfig != null) {
             // The authentication is already in progress:
@@ -200,7 +223,14 @@ export class CodexAcpClient {
     }
 
     async resumeSession(request: acp.ResumeSessionRequest): Promise<SessionMetadata> {
-        await this.refreshSkills(request.cwd, request._meta);
+        const additionalRootPaths = readAdditionalRootPaths(request._meta, request.additionalDirectories);
+        await this.refreshSkills(request.cwd);
+        await installAdditionalRootSkillMarketplaces({
+            codexClient: this.codexClient,
+            cwd: request.cwd,
+            additionalRootPaths,
+        });
+        this.additionalRootPathsBySessionId.set(request.sessionId, additionalRootPaths);
 
         const response = await this.codexClient.threadResume({
             config: await this.createSessionConfig(request.cwd, request.mcpServers ?? []),
@@ -219,6 +249,15 @@ export class CodexAcpClient {
     }
 
     async loadSession(request: acp.LoadSessionRequest): Promise<SessionMetadataWithThread> {
+        const additionalRootPaths = readAdditionalRootPaths(request._meta, request.additionalDirectories);
+        await this.refreshSkills(request.cwd);
+        await installAdditionalRootSkillMarketplaces({
+            codexClient: this.codexClient,
+            cwd: request.cwd,
+            additionalRootPaths,
+        });
+        this.additionalRootPathsBySessionId.set(request.sessionId, additionalRootPaths);
+
         const response = await this.codexClient.threadResume({
             config: await this.createSessionConfig(request.cwd, request.mcpServers ?? []),
             cwd: request.cwd,
@@ -237,7 +276,13 @@ export class CodexAcpClient {
     }
 
     async newSession(request: acp.NewSessionRequest): Promise<SessionMetadata> {
-        await this.refreshSkills(request.cwd, request._meta);
+        const additionalRootPaths = readAdditionalRootPaths(request._meta, request.additionalDirectories);
+        await this.refreshSkills(request.cwd);
+        await installAdditionalRootSkillMarketplaces({
+            codexClient: this.codexClient,
+            cwd: request.cwd,
+            additionalRootPaths,
+        });
 
         const response = await this.codexClient.threadStart({
             config: await this.createSessionConfig(request.cwd, request.mcpServers),
@@ -250,6 +295,7 @@ export class CodexAcpClient {
             throw new Error("Codex did not return any models");
         }
         const currentModelId = this.createModelId(codexModels, response.model, response.reasoningEffort).toString();
+        this.additionalRootPathsBySessionId.set(response.thread.id, additionalRootPaths);
         return {
             sessionId: response.thread.id,
             currentModelId: currentModelId,
@@ -312,18 +358,13 @@ export class CodexAcpClient {
         return this.getModelProvider() ?? "openai";
     }
 
-    private async refreshSkills(cwd: string, meta?: Record<string, unknown> | null): Promise<void> {
+    private async refreshSkills(cwd: string): Promise<void> {
         if (!cwd) {
             return;
         }
-        const additionalRoots = readAdditionalRoots(meta);
         await this.codexClient.listSkills({
             cwds: [cwd],
-            forceReload: true,
-            perCwdExtraUserRoots: [{
-                cwd: cwd,
-                extraUserRoots: additionalRoots
-            }]
+            forceReload: true
         });
     }
 
@@ -389,7 +430,17 @@ export class CodexAcpClient {
         const input = buildPromptItems(request.prompt);
         const effort = modelId.effort as ReasoningEffort | null; //TODO remove unsafe conversion
 
-        await this.refreshSkills(cwd, request._meta);
+        const additionalRootPaths = mergeAdditionalRootPaths(
+            this.additionalRootPathsBySessionId.get(request.sessionId) ?? [],
+            readAdditionalRootPaths(request._meta)
+        );
+        this.additionalRootPathsBySessionId.set(request.sessionId, additionalRootPaths);
+        await this.refreshSkills(cwd);
+        await installAdditionalRootSkillMarketplaces({
+            codexClient: this.codexClient,
+            cwd,
+            additionalRootPaths,
+        });
         return await this.codexClient.runTurn({
             threadId: request.sessionId,
             input: input,
@@ -584,8 +635,13 @@ function buildPromptItems(prompt: acp.ContentBlock[]): UserInput[] {
                 const url = block.uri ?? `data:${block.mimeType};base64,${block.data}`;
                 return {type: "image", url};
             }
-            case "resource_link":
+            case "resource_link": {
+                const skillInput = buildSkillInput(block);
+                if (skillInput !== null) {
+                    return skillInput;
+                }
                 return {type: "text", text: formatUriAsLink(block.name, block.uri), text_elements: []};
+            }
             case "resource": {
                 const resource = block.resource as EmbeddedResourceResource;
                 if ("text" in resource) {
@@ -605,14 +661,51 @@ function formatUriAsLink(name: string | null | undefined, uri: string): string {
     if (name && name.length > 0) {
         return `[@${name}](${uri})`;
     }
-    if (uri.startsWith("file://")) {
-        const path = uri.replace("file://", "");
+    if (uri.startsWith(FILE_URI_PREFIX)) {
+        const path = uri.replace(FILE_URI_PREFIX, "");
         const fileName = path.split("/").pop() ?? path;
         return `[@${fileName}](${uri})`;
     }
     return uri;
 }
 
+function buildSkillInput(block: acp.ResourceLink): UserInput | null {
+    const skillPath = parseSkillPath(block.uri);
+    if (skillPath === null) {
+        return null;
+    }
+
+    const name = readSkillName(block, skillPath);
+    if (name === null) {
+        return null;
+    }
+
+    return {type: "skill", name, path: skillPath};
+}
+
+function parseSkillPath(uri: string): string | null {
+    if (!uri.startsWith(FILE_URI_PREFIX)) {
+        return null;
+    }
+
+    let filePath: string;
+    try {
+        filePath = fileURLToPath(uri);
+    } catch {
+        return null;
+    }
+
+    return path.basename(filePath) === CODEX_SKILL_FILE_NAME ? filePath : null;
+}
+
+function readSkillName(block: acp.ResourceLink, skillPath: string): string | null {
+    const rawName = block.name === CODEX_SKILL_FILE_NAME
+        ? path.basename(path.dirname(skillPath))
+        : block.name;
+    const name = rawName.trim().replace(/^\$/, "");
+    return name.length > 0 ? name : null;
+}
+
 interface GatewayConfig {
     modelProvider: string;
     config: {
@@ -623,13 +716,21 @@ interface GatewayConfig {
     }
 }
 
-function readAdditionalRoots(meta: Record<string, unknown> | null | undefined): string[] {
+function readAdditionalRootPaths(
+    meta: Record<string, unknown> | null | undefined,
+    additionalDirectories: AdditionalRootPaths | undefined = undefined
+): AdditionalRootPaths {
     const rawRoots = meta?.["additionalRoots"];
-    if (!Array.isArray(rawRoots)) {
-        return [];
-    }
+    const metaRoots = Array.isArray(rawRoots) ? rawRoots : [];
+    return normalizeAdditionalRootPaths([...metaRoots, ...(additionalDirectories ?? [])]);
+}
+
+function mergeAdditionalRootPaths(left: AdditionalRootPaths, right: AdditionalRootPaths): AdditionalRootPaths {
+    return normalizeAdditionalRootPaths([...left, ...right]);
+}
 
-    return Array.from(new Set(rawRoots
+function normalizeAdditionalRootPaths(values: unknown[]): AdditionalRootPaths {
+    return Array.from(new Set(values
         .filter((value): value is string => typeof value === "string")
         .map(value => value.trim())
         .filter(value => value.length > 0)));