diff --git a/package-lock.json b/package-lock.json index 9c64924..37e028d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,8 @@ "@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" }, "bin": { "codex-acp": "dist/index.js" @@ -1284,6 +1285,7 @@ "integrity": "sha512-GNWcUTRBgIRJD5zj+Tq0fKOJ5XZajIiBroOF0yvj2bSU1WvNdYS/dn9UxwsujGW4JX06dnHyjV2y9rRaybH0iQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1930,6 +1932,7 @@ "integrity": "sha512-F2X8g9P1X7uCPZMA3MVf9wcTqlyNp7IhH5qPCI0izhaOIYXaW9L535tGA3qmjRzpH+bZczqq7hVKxTR4NWnu+g==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", @@ -2632,6 +2635,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -2923,8 +2927,7 @@ "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.27.0.tgz", "integrity": "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/send": { "version": "0.19.2", @@ -3188,6 +3191,7 @@ "integrity": "sha512-ytQKuwgmrrkDTFP4LjR0ToE2nqgy886GpvRSpU0JAnrdBYppuY5rLkRUYPU1yCryb24SsKBTL/hlDQAEFVwtZg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -3273,6 +3277,7 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -3485,11 +3490,28 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/yaml": { + "version": "2.9.0", + "resolved": "https://registry.npmjs.org/yaml/-/yaml-2.9.0.tgz", + "integrity": "sha512-2AvhNX3mb8zd6Zy7INTtSpl1F15HW6Wnqj0srWlkKLcpYl/gMIMJiyuGq2KeI2YFxUPjdlB+3Lc10seMLtL4cA==", + "license": "ISC", + "peer": true, + "bin": { + "yaml": "bin.mjs" + }, + "engines": { + "node": ">= 14.6" + }, + "funding": { + "url": "https://github.com/sponsors/eemeli" + } + }, "node_modules/zod": { "version": "3.25.76", "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/package.json b/package.json index 9c35aa8..f9e4a82 100644 --- a/package.json +++ b/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" } } diff --git a/src/AcpExtensions.ts b/src/AcpExtensions.ts index 1378013..70dfb00 100644 --- a/src/AcpExtensions.ts +++ b/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 }): 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 = {} \ No newline at end of file +export type AuthenticationLogoutResponse = {} diff --git a/src/CodexAcpClient.ts b/src/CodexAcpClient.ts index 22cde0d..e8f3842 100644 --- a/src/CodexAcpClient.ts +++ b/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(); private pendingLoginCompleted: Promise | null = null; private pendingAccountUpdated: Promise | null = null; @@ -182,6 +194,17 @@ export class CodexAcpClient { await accountUpdatedPromise; } + async removeMarketplace(marketplaceName: string): Promise { + await this.codexClient.marketplaceRemove({marketplaceName}); + } + + async listMarketplaces(cwd: string): Promise { + const pluginList = await this.codexClient.pluginList({ + cwds: cwd ? [cwd] : [] + }); + return pluginList.marketplaces.map((marketplace) => marketplace.name); + } + async authRequired(): Promise { if (this.gatewayConfig != null) { // The authentication is already in progress: @@ -200,7 +223,14 @@ export class CodexAcpClient { } async resumeSession(request: acp.ResumeSessionRequest): Promise { - 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 { + 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 { - 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 | null): Promise { + private async refreshSkills(cwd: string): Promise { 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 | null | undefined): string[] { +function readAdditionalRootPaths( + meta: Record | 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))); diff --git a/src/CodexAppServerClient.ts b/src/CodexAppServerClient.ts index 0d2d083..c79f1a1 100644 --- a/src/CodexAppServerClient.ts +++ b/src/CodexAppServerClient.ts @@ -12,6 +12,8 @@ import type { GetAccountResponse, ListMcpServerStatusParams, ListMcpServerStatusResponse, + MarketplaceAddParams, + MarketplaceAddResponse, LoginAccountParams, LoginAccountResponse, LogoutAccountResponse, @@ -21,6 +23,12 @@ import type { McpServerStatusUpdatedNotification, ModelListParams, ModelListResponse, + PluginInstallParams, + PluginInstallResponse, + PluginListParams, + PluginListResponse, + PluginReadParams, + PluginReadResponse, SkillsListParams, SkillsListResponse, ThreadLoadedListParams, @@ -42,6 +50,8 @@ import type { CommandExecutionRequestApprovalResponse, FileChangeRequestApprovalParams, FileChangeRequestApprovalResponse, + MarketplaceRemoveParams, + MarketplaceRemoveResponse, } from "./app-server/v2"; export interface ApprovalHandler { @@ -264,6 +274,26 @@ export class CodexAppServerClient { return await this.sendRequest({ method: "skills/list", params }); } + async marketplaceAdd(params: MarketplaceAddParams): Promise { + return await this.sendRequest({ method: "marketplace/add", params }); + } + + async marketplaceRemove(params: MarketplaceRemoveParams): Promise { + return await this.sendRequest({ method: "marketplace/remove", params }); + } + + async pluginList(params: PluginListParams): Promise { + return await this.sendRequest({ method: "plugin/list", params }); + } + + async pluginRead(params: PluginReadParams): Promise { + return await this.sendRequest({ method: "plugin/read", params }); + } + + async pluginInstall(params: PluginInstallParams): Promise { + return await this.sendRequest({ method: "plugin/install", params }); + } + /** * Registers a notification handler for a specific session. * Replaces any existing handler for the same session, preventing handler accumulation. diff --git a/src/LocalSkillMarketplace.ts b/src/LocalSkillMarketplace.ts new file mode 100644 index 0000000..a43a4ef --- /dev/null +++ b/src/LocalSkillMarketplace.ts @@ -0,0 +1,654 @@ +import {promises as fs} from "node:fs"; +import path from "node:path"; +import type {CodexAppServerClient} from "./CodexAppServerClient"; +import {CODEX_SKILL_FILE_NAME} from "./SkillDirectoryParser"; +import {logger} from "./Logger"; +import {parseSkillsFromDirectory} from "./SkillDirectoryParser"; +import {formatError, isNotFoundError, isRecord} from "./TypeGuards"; + +const AGENTS_DIR = ".agents"; +const SKILLS_DIR = "skills"; +const PLUGINS_DIR = "plugins"; +const GENERATED_DIR = "generated"; +const SYNC_METADATA_FILE_NAME = ".codex-acp-sync.json"; + +// Codex app-server 0.130.0 removed the session/turn `additionalDirectories` +// field. ACP clients can still provide additional roots, so we preserve that +// behavior by turning each provided directory's `.agents/skills` folder into a +// local skills-only plugin marketplace and installing it through app-server. + +// Public result consumed by plugin registration/install helpers. +export type LocalSkillMarketplaceResult = + | { + ok: true; + marketplacePath: string; + marketplaceName: string; + pluginName: string; + skills: GeneratedSkillMetadata[]; + changed: boolean; + } + | { + ok: false; + error: string; + }; + +// Shape of /.agents/plugins/marketplace.json. +type MarketplaceManifest = { + name: string; + interface?: { + displayName?: string | null; + }; + plugins: MarketplacePluginEntry[]; +}; + +// Single catalog entry pointing app-server at the generated skills-only plugin. +type MarketplacePluginEntry = { + name: string; + source: { + source: "local"; + path: string; + }; + policy: { + installation: "AVAILABLE"; + authentication: "ON_INSTALL"; + }; + category: "Productivity"; +}; + +// Parsed source skill plus the generated destination paths derived from it. +type SourceSkillDirectory = { + sourceDir: string; + skillName: string; + generatedSkillDir: string; + generatedSkillPath: string; +}; + +// Generated skill identity used for app-server validation and sync freshness checks. +export type GeneratedSkillMetadata = { + name: string; + sourceDir: string; + generatedSkillDir: string; + generatedSkillPath: string; + sourceLatestMtimeMs: number; +}; + +// Plugin-level cache of source mtimes, so unchanged generated skills can be skipped. +type PluginSyncMetadata = { + sourceRoot: string; + pluginName: string; + syncedAt: string; + skills: Record; +}; + +// Per-skill sync state keyed by generated skill directory relative to generatedSkillsRoot. +type PluginSyncSkillMetadata = { + sourceDir: string; + generatedSkillPath: string; + sourceLatestMtimeMs: number; +}; + +// Internal sync outcome used to decide whether app-server must reinstall the plugin. +type SkillSyncSummary = { + copied: string[]; + unchanged: string[]; + removed: string[]; + changed: boolean; +}; + +export async function createMarketplaceFromSkills( + inputPath: string +): Promise { + let root: string; + try { + root = await canonicalizeExistingDirectory(inputPath); + } catch (error) { + const message = `Invalid additional root ${inputPath}: ${formatError(error)}`; + logger.log("Local skill marketplace generation skipped", {root: inputPath, error: message}); + return {ok: false, error: message}; + } + + const skillsRoot = path.join(root, AGENTS_DIR, SKILLS_DIR); + const parsed = await parseSkillsFromDirectory(skillsRoot); + if (parsed.errors.length > 0) { + logger.log("Local skill marketplace skill parse errors", { + root, + errors: parsed.errors, + }); + } + if (parsed.skills.length === 0) { + const message = `No valid skills found under ${skillsRoot}`; + logger.log("Local skill marketplace generation skipped", {root, error: message}); + return {ok: false, error: message}; + } + + const marketplaceName = deriveMarketplaceName(root); + if (marketplaceName === null) { + const message = `Could not derive marketplace name from ${root}`; + logger.log("Local skill marketplace generation skipped", {root, error: message}); + return {ok: false, error: message}; + } + + const pluginName = `${marketplaceName}-skills`; + const marketplacePath = path.join(root, AGENTS_DIR, PLUGINS_DIR, "marketplace.json"); + const pluginRoot = path.join(root, AGENTS_DIR, PLUGINS_DIR, GENERATED_DIR, pluginName); + const generatedSkillsRoot = path.join(pluginRoot, SKILLS_DIR); + const sourceSkillDirectories = parsed.skills.map((skill): SourceSkillDirectory => { + const sourceDir = path.dirname(skill.path); + const generatedRelativeDir = path.relative(skillsRoot, sourceDir); + const generatedSkillDir = path.join(generatedSkillsRoot, generatedRelativeDir); + return { + sourceDir, + skillName: skill.name, + generatedSkillDir, + generatedSkillPath: path.join(generatedSkillDir, CODEX_SKILL_FILE_NAME), + }; + }); + let syncedSkills: GeneratedSkillMetadata[]; + let syncSummary: SkillSyncSummary; + + try { + syncedSkills = await Promise.all(sourceSkillDirectories.map(async (skill) => ({ + name: skill.skillName, + sourceDir: skill.sourceDir, + generatedSkillDir: skill.generatedSkillDir, + generatedSkillPath: skill.generatedSkillPath, + sourceLatestMtimeMs: await readLatestSourceMtimeMs(skill.sourceDir), + }))); + await fs.mkdir(generatedSkillsRoot, {recursive: true}); + syncSummary = await syncSkillDirectories({ + sourceRoot: root, + pluginName, + pluginRoot, + generatedSkillsRoot, + skills: syncedSkills, + }); + await writePluginManifest(pluginRoot, pluginName, root); + await ensureMarketplaceEntry({ + marketplacePath, + marketplaceName, + pluginName, + pluginSourcePath: `./${AGENTS_DIR}/${PLUGINS_DIR}/${GENERATED_DIR}/${pluginName}`, + }); + } catch (error) { + const message = `Failed to generate local skill marketplace for ${root}: ${formatError(error)}`; + logger.error("Local skill marketplace generation failed", error); + return {ok: false, error: message}; + } + + return { + ok: true, + marketplacePath, + marketplaceName, + pluginName, + skills: syncedSkills, + changed: syncSummary.changed, + }; +} + +export async function installAdditionalRootSkillMarketplaces(params: { + codexClient: CodexAppServerClient; + cwd: string; + additionalRootPaths: string[]; +}): Promise { + // Workflow: + // additional root paths -> parse .agents/skills -> generate marketplace.json + // -> sync generated plugin skills -> write plugin manifest -> marketplace/add + // -> plugin/list -> plugin/read diff -> plugin/install when stale or unavailable. + const {codexClient, cwd, additionalRootPaths} = params; + for (const additionalRootPath of additionalRootPaths) { + const marketplace = await createMarketplaceFromSkills(additionalRootPath); + if (!marketplace.ok) { + logger.log("Additional root skill marketplace skipped", { + root: additionalRootPath, + error: marketplace.error, + }); + continue; + } + await installGeneratedSkillPlugin({ + codexClient, + cwd, + root: additionalRootPath, + marketplace, + }); + } +} + +async function installGeneratedSkillPlugin(params: { + codexClient: CodexAppServerClient; + cwd: string; + root: string; + marketplace: Extract; +}): Promise { + const {codexClient, cwd, root, marketplace} = params; + try { + const marketplaceAdd = await codexClient.marketplaceAdd({ + source: root, + refName: null, + sparsePaths: null, + }); + const pluginList = await codexClient.pluginList({ + cwds: cwd ? [cwd] : [] + }); + const listedMarketplace = pluginList.marketplaces.find((entry) => + entry.name === marketplace.marketplaceName || entry.path === marketplace.marketplacePath + ); + if (!listedMarketplace) { + logger.log("Generated skill marketplace was not listed after add", { + root, + marketplaceName: marketplace.marketplaceName, + marketplacePath: marketplace.marketplacePath, + installedRoot: marketplaceAdd.installedRoot, + }); + return; + } + + const plugin = listedMarketplace.plugins.find((candidate) => + candidate.name === marketplace.pluginName + ); + if (!plugin) { + logger.log("Generated skill plugin was not listed after marketplace add", { + root, + marketplaceName: marketplace.marketplaceName, + pluginName: marketplace.pluginName, + }); + return; + } + + const marketplacePath = listedMarketplace.path ?? marketplace.marketplacePath; + let skillDiff = await readGeneratedPluginSkillDiff( + codexClient, + marketplacePath, + marketplace.pluginName, + marketplace.skills.map((skill) => skill.generatedSkillPath) + ); + const shouldRefreshInstall = !plugin.installed + || !plugin.enabled + || marketplace.changed + || skillDiff.missing.length > 0 + || skillDiff.extra.length > 0; + if (skillDiff.missing.length > 0 || skillDiff.extra.length > 0) { + logger.log("Generated skill plugin contents differ from source before install", { + root, + marketplaceName: marketplace.marketplaceName, + pluginName: marketplace.pluginName, + missingSkillPaths: skillDiff.missing, + extraSkillPaths: skillDiff.extra, + expectedSkills: marketplace.skills.map((skill) => ({ + name: skill.name, + path: skill.generatedSkillPath, + })), + }); + } + + if (shouldRefreshInstall) { + await codexClient.pluginInstall({ + marketplacePath, + remoteMarketplaceName: null, + pluginName: marketplace.pluginName, + }); + } + + logger.log("Additional root skill plugin installed", { + root, + marketplaceName: marketplace.marketplaceName, + pluginName: marketplace.pluginName, + alreadyAdded: marketplaceAdd.alreadyAdded, + generatedPluginChanged: marketplace.changed, + expectedSkillCount: marketplace.skills.length, + listedSkillCount: skillDiff.actual.length, + }); + } catch (error) { + logger.error("Additional root skill plugin install failed", error); + } +} + +async function readGeneratedPluginSkillDiff( + codexClient: CodexAppServerClient, + marketplacePath: string, + pluginName: string, + expectedSkillPaths: string[] +): Promise<{actual: string[]; missing: string[]; extra: string[]}> { + const pluginRead = await codexClient.pluginRead({ + marketplacePath, + remoteMarketplaceName: null, + pluginName, + }); + const actualSkillPaths = sortedUnique(pluginRead.plugin.skills + .map((skill) => skill.path) + .filter((skillPath): skillPath is string => skillPath !== null)); + return diffSortedValues(sortedUnique(expectedSkillPaths), actualSkillPaths); +} + +async function canonicalizeExistingDirectory(directory: string): Promise { + const resolved = path.resolve(directory); + const stat = await fs.stat(resolved); + if (!stat.isDirectory()) { + throw new Error("path is not a directory"); + } + return await fs.realpath(resolved); +} + +function deriveMarketplaceName(root: string): string | null { + const normalized = path.normalize(root); + const parts = normalized.split(path.sep).filter((part) => part.length > 0); + const selected = parts.slice(-2).map(normalizeMarketplaceSegment).filter((part) => part.length > 0); + const marketplaceName = selected.join("-"); + return marketplaceName.length > 0 ? marketplaceName : null; +} + +function normalizeMarketplaceSegment(segment: string): string { + return segment.toLowerCase().replace(/[^a-z0-9]+/g, "-").replace(/^-+|-+$/g, ""); +} + +function diffSortedValues(expected: string[], actual: string[]): {actual: string[]; missing: string[]; extra: string[]} { + const actualSet = new Set(actual); + const expectedSet = new Set(expected); + return { + actual, + missing: expected.filter((name) => !actualSet.has(name)), + extra: actual.filter((name) => !expectedSet.has(name)), + }; +} + +function sortedUnique(values: string[]): string[] { + return Array.from(new Set(values)).sort((left, right) => left.localeCompare(right)); +} + +async function syncSkillDirectories(params: { + sourceRoot: string; + pluginName: string; + pluginRoot: string; + generatedSkillsRoot: string; + skills: GeneratedSkillMetadata[]; +}): Promise { + const {sourceRoot, pluginName, pluginRoot, generatedSkillsRoot, skills} = params; + await fs.mkdir(generatedSkillsRoot, {recursive: true}); + const parsedExistingMetadata = await readPluginSyncMetadata(pluginRoot); + const existingMetadata = parsedExistingMetadata?.sourceRoot === sourceRoot + && parsedExistingMetadata.pluginName === pluginName + ? parsedExistingMetadata + : null; + const expectedGeneratedNames = new Set(); + const copiedSkillNames = new Set(); + const nextMetadata: PluginSyncMetadata = { + sourceRoot, + pluginName, + syncedAt: new Date().toISOString(), + skills: {}, + }; + const syncSummary = { + copied: [] as string[], + unchanged: [] as string[], + removed: [] as string[], + changed: false, + }; + + for (const skillDir of skills) { + const generatedRelativeDir = path.relative(generatedSkillsRoot, skillDir.generatedSkillDir); + // Defensive containment check before removing/copying: generated skill destinations + // must stay under the generated plugin's skills directory. + if (generatedRelativeDir.startsWith("..") || path.isAbsolute(generatedRelativeDir) || copiedSkillNames.has(generatedRelativeDir)) { + continue; + } + copiedSkillNames.add(generatedRelativeDir); + expectedGeneratedNames.add(generatedRelativeDir); + + const destination = skillDir.generatedSkillDir; + const metadataEntry = existingMetadata?.skills[generatedRelativeDir]; + const generatedSkillExists = await pathExists(skillDir.generatedSkillPath); + if (isSkillUnchanged(metadataEntry, skillDir) && generatedSkillExists) { + syncSummary.unchanged.push(generatedRelativeDir); + nextMetadata.skills[generatedRelativeDir] = metadataEntry; + continue; + } + + await fs.rm(destination, {recursive: true, force: true}); + await fs.cp(skillDir.sourceDir, destination, {recursive: true, force: true}); + nextMetadata.skills[generatedRelativeDir] = createPluginSyncSkillMetadata(skillDir); + syncSummary.copied.push(generatedRelativeDir); + } + + syncSummary.removed = await pruneStaleGeneratedSkills(generatedSkillsRoot, expectedGeneratedNames); + syncSummary.changed = syncSummary.copied.length > 0 || syncSummary.removed.length > 0; + await writePluginSyncMetadata(pluginRoot, nextMetadata); + logger.log("Local skill marketplace sync completed", syncSummary); + return syncSummary; +} + +function isSkillUnchanged( + metadataEntry: PluginSyncSkillMetadata | undefined, + skill: GeneratedSkillMetadata +): metadataEntry is PluginSyncSkillMetadata { + return metadataEntry !== undefined + && metadataEntry.sourceDir === skill.sourceDir + && metadataEntry.generatedSkillPath === skill.generatedSkillPath + && metadataEntry.sourceLatestMtimeMs === skill.sourceLatestMtimeMs; +} + +function createPluginSyncSkillMetadata( + skill: GeneratedSkillMetadata +): PluginSyncSkillMetadata { + return { + sourceDir: skill.sourceDir, + generatedSkillPath: skill.generatedSkillPath, + sourceLatestMtimeMs: skill.sourceLatestMtimeMs, + }; +} + +async function readLatestSourceMtimeMs(directory: string): Promise { + let latestMtimeMs = 0; + const queue = [directory]; + while (queue.length > 0) { + const currentDirectory = queue.shift()!; + const directoryStat = await fs.stat(currentDirectory); + latestMtimeMs = Math.max(latestMtimeMs, directoryStat.mtimeMs); + const entries = await fs.readdir(currentDirectory, {withFileTypes: true}); + for (const entry of entries) { + if (entry.name.startsWith(".")) { + continue; + } + const entryPath = path.join(currentDirectory, entry.name); + if (entry.isDirectory()) { + queue.push(entryPath); + continue; + } + if (entry.isFile() || entry.isSymbolicLink()) { + const stat = await fs.stat(entryPath); + latestMtimeMs = Math.max(latestMtimeMs, stat.mtimeMs); + } + } + } + return latestMtimeMs; +} + +async function readPluginSyncMetadata(pluginRoot: string): Promise { + const metadataPath = path.join(pluginRoot, SYNC_METADATA_FILE_NAME); + let contents: string; + try { + contents = await fs.readFile(metadataPath, "utf8"); + } catch (error) { + if (isNotFoundError(error)) { + return null; + } + throw error; + } + + let parsed: unknown; + try { + parsed = JSON.parse(contents); + } catch { + return null; + } + if (!isPluginSyncMetadata(parsed)) { + return null; + } + return parsed; +} + +async function writePluginSyncMetadata(pluginRoot: string, metadata: PluginSyncMetadata): Promise { + await fs.writeFile( + path.join(pluginRoot, SYNC_METADATA_FILE_NAME), + `${JSON.stringify(metadata, null, 2)}\n`, + "utf8" + ); +} + +async function pruneStaleGeneratedSkills( + generatedSkillsRoot: string, + expectedGeneratedNames: Set +): Promise { + const removed: string[] = []; + const generatedSkillDirs = await readGeneratedSkillDirs(generatedSkillsRoot); + for (const generatedSkillDir of generatedSkillDirs) { + const relativeDir = path.relative(generatedSkillsRoot, generatedSkillDir); + if (expectedGeneratedNames.has(relativeDir)) { + continue; + } + await fs.rm(generatedSkillDir, {recursive: true, force: true}); + removed.push(relativeDir); + } + return removed; +} + +async function readGeneratedSkillDirs(generatedSkillsRoot: string): Promise { + const skillDirs: string[] = []; + const queue = [generatedSkillsRoot]; + while (queue.length > 0) { + const currentDirectory = queue.shift()!; + const entries = await fs.readdir(currentDirectory, {withFileTypes: true}); + for (const entry of entries) { + if (entry.name.startsWith(".")) { + continue; + } + const entryPath = path.join(currentDirectory, entry.name); + if (entry.isDirectory()) { + queue.push(entryPath); + continue; + } + if (entry.isFile() && entry.name === CODEX_SKILL_FILE_NAME) { + skillDirs.push(currentDirectory); + } + } + } + return skillDirs; +} + +async function writePluginManifest(pluginRoot: string, pluginName: string, sourceRoot: string): Promise { + const manifestPath = path.join(pluginRoot, ".codex-plugin", "plugin.json"); + await fs.mkdir(path.dirname(manifestPath), {recursive: true}); + const manifest = { + name: pluginName, + version: "1.0.0", + description: `Generated skills-only plugin from ${sourceRoot}.`, + skills: "./skills/", + }; + await fs.writeFile(manifestPath, `${JSON.stringify(manifest, null, 2)}\n`, "utf8"); +} + +async function ensureMarketplaceEntry(params: { + marketplacePath: string; + marketplaceName: string; + pluginName: string; + pluginSourcePath: string; +}): Promise { + const manifest = await readMarketplaceManifest(params.marketplacePath, params.marketplaceName); + const entry = createMarketplacePluginEntry(params.pluginName, params.pluginSourcePath); + const existingIndex = manifest.plugins.findIndex((plugin) => plugin.name === params.pluginName); + if (existingIndex >= 0) { + manifest.plugins[existingIndex] = entry; + } else { + manifest.plugins.push(entry); + } + + await fs.mkdir(path.dirname(params.marketplacePath), {recursive: true}); + await fs.writeFile(params.marketplacePath, `${JSON.stringify(manifest, null, 2)}\n`, "utf8"); +} + +async function readMarketplaceManifest( + marketplacePath: string, + marketplaceName: string +): Promise { + let contents: string; + try { + contents = await fs.readFile(marketplacePath, "utf8"); + } catch (error) { + if (isNotFoundError(error)) { + return createMarketplaceManifest(marketplaceName); + } + throw error; + } + + let parsed: unknown; + try { + parsed = JSON.parse(contents); + } catch (error) { + throw new Error(`invalid marketplace JSON at ${marketplacePath}: ${formatError(error)}`); + } + + if (!isMarketplaceManifest(parsed)) { + throw new Error(`invalid marketplace manifest shape at ${marketplacePath}`); + } + + return parsed; +} + +function createMarketplaceManifest(marketplaceName: string): MarketplaceManifest { + return { + name: marketplaceName, + interface: { + displayName: marketplaceName, + }, + plugins: [], + }; +} + +function createMarketplacePluginEntry(pluginName: string, pluginSourcePath: string): MarketplacePluginEntry { + return { + name: pluginName, + source: { + source: "local", + path: pluginSourcePath, + }, + policy: { + installation: "AVAILABLE", + authentication: "ON_INSTALL", + }, + category: "Productivity", + }; +} + +function isMarketplaceManifest(value: unknown): value is MarketplaceManifest { + if (!isRecord(value) || typeof value["name"] !== "string" || !Array.isArray(value["plugins"])) { + return false; + } + const marketplaceInterface = value["interface"]; + return marketplaceInterface === undefined || marketplaceInterface === null || isRecord(marketplaceInterface); +} + +function isPluginSyncMetadata(value: unknown): value is PluginSyncMetadata { + return isRecord(value) + && typeof value["sourceRoot"] === "string" + && typeof value["pluginName"] === "string" + && typeof value["syncedAt"] === "string" + && isRecord(value["skills"]) + && Object.values(value["skills"]).every(isPluginSyncSkillMetadata); +} + +function isPluginSyncSkillMetadata(value: unknown): value is PluginSyncSkillMetadata { + return isRecord(value) + && typeof value["sourceDir"] === "string" + && typeof value["generatedSkillPath"] === "string" + && typeof value["sourceLatestMtimeMs"] === "number"; +} + +async function pathExists(filePath: string): Promise { + try { + await fs.stat(filePath); + return true; + } catch (error) { + if (isNotFoundError(error)) { + return false; + } + throw error; + } +} diff --git a/src/SkillDirectoryParser.ts b/src/SkillDirectoryParser.ts new file mode 100644 index 0000000..5aaee61 --- /dev/null +++ b/src/SkillDirectoryParser.ts @@ -0,0 +1,266 @@ +import {promises as fs} from "node:fs"; +import type {Dirent} from "node:fs"; +import path from "node:path"; +import {parse as parseYaml} from "yaml"; +import {formatError, isNotFoundError, isRecord} from "./TypeGuards"; + +export const CODEX_SKILL_FILE_NAME = "SKILL.md"; +export const CODEX_SKILL_METADATA_DIR_NAME = "agents"; +export const CODEX_SKILL_METADATA_FILE_NAME = "openai.yaml"; +export const CODEX_SKILL_MAX_SCAN_DEPTH = 6; + +export type ParsedSkillInterface = { + displayName?: string; + shortDescription?: string; + brandColor?: string; + defaultPrompt?: string; +}; + +export type ParsedSkill = { + name: string; + description: string; + shortDescription?: string; + interface?: ParsedSkillInterface; + path: string; + scope: "user"; + enabled: true; +}; + +export type ParsedSkillError = { + path: string; + message: string; +}; + +export type ParsedSkillLoadResult = { + skills: ParsedSkill[]; + errors: ParsedSkillError[]; +}; + +type QueueEntry = { + directory: string; + depth: number; +}; + +/** + * Reads Codex skills from a local skills directory using the same core filesystem conventions as + * codex app-server: skills are declared by files named exactly `SKILL.md`, with metadata in YAML + * frontmatter and optional UI metadata in `agents/openai.yaml`. + */ +export async function parseSkillsFromDirectory(rootDir: string): Promise { + const root = path.resolve(rootDir); + const skills: ParsedSkill[] = []; + const errors: ParsedSkillError[] = []; + const queue: QueueEntry[] = [{directory: root, depth: 0}]; + const visitedDirectories = new Set(); + + while (queue.length > 0) { + const entry = queue.shift()!; + const directory = await canonicalizePath(entry.directory); + if (visitedDirectories.has(directory)) { + continue; + } + visitedDirectories.add(directory); + + let children: Dirent[]; + try { + children = await fs.readdir(directory, {withFileTypes: true}); + } catch (err) { + if (entry.depth === 0 && isNotFoundError(err)) { + return {skills, errors}; + } + errors.push({ + path: directory, + message: `failed to read skills dir: ${formatError(err)}`, + }); + continue; + } + + for (const child of children) { + if (child.name.startsWith(".")) { + continue; + } + + const childPath = path.join(directory, child.name); + if (child.isDirectory()) { + enqueueDirectory(queue, childPath, entry.depth + 1); + continue; + } + + if (child.isSymbolicLink()) { + await enqueueSymlinkedDirectory(queue, childPath, entry.depth + 1); + continue; + } + + if (!child.isFile() || child.name !== CODEX_SKILL_FILE_NAME) { + continue; + } + + try { + skills.push(await parseSkillFile(childPath)); + } catch (err) { + errors.push({ + path: childPath, + message: formatError(err), + }); + } + } + } + + skills.sort((left, right) => { + const byName = left.name.localeCompare(right.name); + return byName !== 0 ? byName : left.path.localeCompare(right.path); + }); + return {skills, errors}; +} + +async function parseSkillFile(skillPath: string): Promise { + const contents = await fs.readFile(skillPath, "utf8"); + const frontmatter = extractFrontmatter(contents); + if (frontmatter === null) { + throw new Error("missing YAML frontmatter delimited by ---"); + } + + const metadata = parseYamlRecord(frontmatter); + const rawName = readYamlString(metadata, ["name"]); + const name = sanitizeSingleLine(rawName ?? path.basename(path.dirname(skillPath))); + const description = sanitizeSingleLine(readYamlString(metadata, ["description"]) ?? ""); + const shortDescription = optionalSanitizedValue(readYamlString(metadata, ["metadata", "short-description"])); + const resolvedPath = await canonicalizePath(skillPath); + const skillInterface = await readSkillInterface(path.dirname(skillPath)); + + return { + name: name.length > 0 ? name : "skill", + description, + ...(shortDescription !== undefined ? {shortDescription} : {}), + ...(skillInterface !== undefined ? {interface: skillInterface} : {}), + path: resolvedPath, + scope: "user", + enabled: true, + }; +} + +function enqueueDirectory(queue: QueueEntry[], directory: string, depth: number): void { + if (depth > CODEX_SKILL_MAX_SCAN_DEPTH) { + return; + } + queue.push({directory, depth}); +} + +async function enqueueSymlinkedDirectory( + queue: QueueEntry[], + symlinkPath: string, + depth: number +): Promise { + if (depth > CODEX_SKILL_MAX_SCAN_DEPTH) { + return; + } + try { + const stat = await fs.stat(symlinkPath); + if (stat.isDirectory()) { + queue.push({directory: symlinkPath, depth}); + } + } catch { + // Codex ignores broken symlinked skill entries during discovery. + } +} + +async function readSkillInterface(skillDir: string): Promise { + const metadataPath = path.join( + skillDir, + CODEX_SKILL_METADATA_DIR_NAME, + CODEX_SKILL_METADATA_FILE_NAME + ); + + let metadata: string; + try { + metadata = await fs.readFile(metadataPath, "utf8"); + } catch { + return undefined; + } + + let metadataRoot: Record; + try { + metadataRoot = parseYamlRecord(metadata); + } catch { + return undefined; + } + + const displayName = optionalSanitizedValue(readYamlString(metadataRoot, ["interface", "display_name"])); + const shortDescription = optionalSanitizedValue(readYamlString(metadataRoot, ["interface", "short_description"])); + const brandColor = optionalSanitizedValue(readYamlString(metadataRoot, ["interface", "brand_color"])); + const defaultPrompt = optionalSanitizedValue(readYamlString(metadataRoot, ["interface", "default_prompt"])); + const skillInterface = { + ...(displayName !== undefined ? {displayName} : {}), + ...(shortDescription !== undefined ? {shortDescription} : {}), + ...(brandColor !== undefined ? {brandColor} : {}), + ...(defaultPrompt !== undefined ? {defaultPrompt} : {}), + }; + + return Object.keys(skillInterface).length > 0 ? skillInterface : undefined; +} + +function parseYamlRecord(contents: string): Record { + const parsed = parseYaml(contents, { + schema: "failsafe", + stringKeys: true, + maxAliasCount: 0, + }); + if (!isRecord(parsed)) { + return {}; + } + return parsed; +} + +function readYamlString(root: Record, pathSegments: string[]): string | undefined { + const dottedKey = pathSegments.join("."); + const directValue = root[dottedKey]; + if (typeof directValue === "string") { + return directValue; + } + + let current: unknown = root; + for (const segment of pathSegments) { + if (!isRecord(current)) { + return undefined; + } + current = current[segment]; + } + return typeof current === "string" ? current : undefined; +} + +function extractFrontmatter(contents: string): string | null { + const lines = contents.split(/\r?\n/); + if (lines[0]?.trim() !== "---") { + return null; + } + + const frontmatterLines: string[] = []; + for (let index = 1; index < lines.length; index += 1) { + const line = lines[index]!; + if (line.trim() === "---") { + return frontmatterLines.length > 0 ? frontmatterLines.join("\n") : null; + } + frontmatterLines.push(line); + } + return null; +} + +function optionalSanitizedValue(value: string | undefined): string | undefined { + if (value === undefined) { + return undefined; + } + const sanitized = sanitizeSingleLine(value); + return sanitized.length > 0 ? sanitized : undefined; +} + +function sanitizeSingleLine(value: string): string { + return value.trim().replace(/\s+/g, " "); +} + +async function canonicalizePath(filePath: string): Promise { + try { + return await fs.realpath(filePath); + } catch { + return path.resolve(filePath); + } +} diff --git a/src/TypeGuards.ts b/src/TypeGuards.ts new file mode 100644 index 0000000..9d283eb --- /dev/null +++ b/src/TypeGuards.ts @@ -0,0 +1,14 @@ +export function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +export function isNotFoundError(error: unknown): boolean { + return typeof error === "object" + && error !== null + && "code" in error + && error.code === "ENOENT"; +} + +export function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts index a66a827..dfe219f 100644 --- a/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts +++ b/src/__tests__/CodexACPAgent/CodexAcpClient.test.ts @@ -215,60 +215,6 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(logoutSpy).toHaveBeenCalledWith({}); }); - it('prefetches session additional skill roots before thread start', async () => { - const mockFixture = createCodexMockTestFixture(); - const codexAcpClient = mockFixture.getCodexAcpClient(); - const codexAppServerClient = mockFixture.getCodexAppServerClient(); - - const listSkillsSpy = vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({ data: [] }); - const threadStartSpy = vi.spyOn(codexAppServerClient, "threadStart").mockResolvedValue({ - thread: { id: "thread-id" } as any, - model: "gpt-5", - modelProvider: "openai", - cwd: "/workspace", - approvalPolicy: "on-request", - sandbox: "workspace-write", - reasoningEffort: "medium", - } as any); - vi.spyOn(codexAppServerClient, "listModels").mockResolvedValue({ - data: [{ - id: "gpt-5", - model: "gpt-5", - upgrade: null, - upgradeInfo: null, - availabilityNux: null, - displayName: "gpt-5", - description: "test model", - hidden: false, - supportedReasoningEfforts: [{ reasoningEffort: "medium", description: "balanced" }], - defaultReasoningEffort: "medium", - inputModalities: ["text"], - supportsPersonality: false, - additionalSpeedTiers: [], - isDefault: true - }], - nextCursor: null - }); - - await codexAcpClient.newSession({ - cwd: "/workspace", - mcpServers: [], - _meta: { - additionalRoots: ["/skills/one", " /skills/two ", 7] - } - }); - - expect(listSkillsSpy).toHaveBeenCalledWith({ - cwds: ["/workspace"], - forceReload: true, - perCwdExtraUserRoots: [{ - cwd: "/workspace", - extraUserRoots: ["/skills/one", "/skills/two"] - }] - }); - expect(listSkillsSpy.mock.invocationCallOrder[0]!).toBeLessThan(threadStartSpy.mock.invocationCallOrder[0]!); - }); - it('waits for typed mcp startup status updates and returns terminal states', async () => { const mockFixture = createCodexMockTestFixture(); const codexAcpClient = mockFixture.getCodexAcpClient(); @@ -358,43 +304,31 @@ describe('ACP server test', { timeout: 40_000 }, () => { expect(session.sessionId).toBe("thread-id"); }); - it('prefetches session additional skill roots before turn start', async () => { + it('passes provided skill resource links as prompt skill items', async () => { const mockFixture = createCodexMockTestFixture(); - const codexAcpAgent = mockFixture.getCodexAcpAgent(); + const codexAcpClient = mockFixture.getCodexAcpClient(); const codexAppServerClient = mockFixture.getCodexAppServerClient(); - const listSkillsSpy = vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({ data: [] }); - const turnStartSpy = vi.spyOn(codexAppServerClient, "turnStart").mockResolvedValue({ - turn: { id: "turn-id", items: [], status: "inProgress", error: null } - } as any); - vi.spyOn(codexAppServerClient, "awaitTurnCompleted").mockResolvedValue({ + vi.spyOn(codexAppServerClient, "listSkills").mockResolvedValue({data: []}); + const runTurnSpy = vi.spyOn(codexAppServerClient, "runTurn").mockResolvedValue({ threadId: "session-id", - turn: { id: "turn-id", items: [], status: "completed", error: null } + turn: createTurn("turn-id", "completed"), } as any); - vi.spyOn(codexAcpAgent, "getSessionState").mockReturnValue(createTestSessionState({ - sessionId: "session-id", - cwd: "/workspace" - })); - - const promptRequest: acp.PromptRequest = { + await codexAcpClient.sendPrompt({ sessionId: "session-id", - prompt: [{ type: "text", text: "Hello" }], - _meta: { - additionalRoots: ["/skills/one", " /skills/two ", 7] - } - }; - await codexAcpAgent.prompt(promptRequest); + prompt: [ + {type: "text", text: "$extra-skill do the work"}, + {type: "resource_link", name: "extra-skill", uri: "file:///skills/extra-skill/SKILL.md"}, + ], + }, AgentMode.DEFAULT_AGENT_MODE, ModelId.create("gpt-5", "medium"), null, false, "/workspace"); - expect(listSkillsSpy).toHaveBeenCalledWith({ - cwds: ["/workspace"], - forceReload: true, - perCwdExtraUserRoots: [{ - cwd: "/workspace", - extraUserRoots: ["/skills/one", "/skills/two"] - }] - }); - expect(listSkillsSpy.mock.invocationCallOrder[0]!).toBeLessThan(turnStartSpy.mock.invocationCallOrder[0]!); + expect(runTurnSpy).toHaveBeenCalledWith(expect.objectContaining({ + input: [ + {type: "text", text: "$extra-skill do the work", text_elements: []}, + {type: "skill", name: "extra-skill", path: "/skills/extra-skill/SKILL.md"}, + ], + })); }); function loadNotifications(){ diff --git a/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json b/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json index 29aa6bd..4ddf4cb 100644 --- a/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json +++ b/src/__tests__/CodexACPAgent/data/send-attachments-turn-start.json @@ -5,13 +5,7 @@ "cwds": [ "/test/cwd" ], - "forceReload": true, - "perCwdExtraUserRoots": [ - { - "cwd": "cwd", - "extraUserRoots": [] - } - ] + "forceReload": true } } { diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts index e0ef576..f97734f 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e-mcp-approval.test.ts @@ -76,7 +76,7 @@ describeE2E("E2E MCP approval tests (configured in session)", () => { async function createMcpSession(): Promise<{ sessionId: string; invocationMarkerPath: string }> { const invocationMarkerPath = path.join(fixture.workspaceDir, `mcp-tool-invocation-${crypto.randomUUID()}.txt`); - const sessionId = (await fixture.createSession([createMcpServer(invocationMarkerPath)])).sessionId; + const sessionId = (await fixture.createSession({mcpServers: [createMcpServer(invocationMarkerPath)]})).sessionId; return {sessionId, invocationMarkerPath}; } diff --git a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts index a098976..64541d6 100644 --- a/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts +++ b/src/__tests__/CodexACPAgent/e2e/acp-e2e.test.ts @@ -11,6 +11,13 @@ import { type SpawnedAgentFixture, } from "./acp-e2e-test-utils"; +const HELLO_WORLD_ADDITIONAL_ROOT_NAME = "hello-world"; +const HELLO_WORLD_SKILL = { + name: "hello", + description: "Use when the user asks for a hello world skill.", + body: "Respond with a concise hello world message and mention that the local plugin skill is active.", +}; + describeE2E("E2E tests", () => { let fixture: SpawnedAgentFixture | null = null; @@ -93,39 +100,43 @@ describeE2E("E2E tests", () => { it("lists a user skill from the CODEX_HOME", async () => { fixture = await createAuthenticatedFixture(); - fixture.writeSkill({ - name: "integration-skill", - description: "Integration skill", - body: "This skill exists only for integration testing.", - }); + fixture.writeSkill(HELLO_WORLD_SKILL); const session = await fixture.createSession(); - await fixture.expectPromptText(session.sessionId, "/skills", (text) => { - expect(text).toContain("Available skills:"); - expect(text).toContain("- integration-skill: Integration skill"); + await expectHelloSkillListed(fixture, session.sessionId); + }); + + it("lists a skill from additionalDirectories", async () => { + fixture = await createAuthenticatedFixture(); + const additionalRoot = await prepareHelloWorldAdditionalRoot(fixture); + const session = await fixture.createSession({ + additionalDirectories: [additionalRoot], }); + + await expectHelloSkillListed(fixture, session.sessionId); }); - // Currently, `additionalRoots` are not propagated when listing skills - it.skip("lists skills from additional session roots", async () => { + it("lists a skill from _meta.additionalRoots", async () => { fixture = await createAuthenticatedFixture(); - const additionalSkillsRoot = path.join(fixture.workspaceDir, "custom-skills"); - fixture.writeSkill({ - name: "session-root-skill", - description: "Session root skill", - body: "This skill exists only in an additional root passed at session creation.", - }, additionalSkillsRoot); - - const session = await fixture.connection.newSession({ - cwd: fixture.workspaceDir, - mcpServers: [], + const additionalRoot = await prepareHelloWorldAdditionalRoot(fixture); + const session = await fixture.createSession({ _meta: { - additionalRoots: [additionalSkillsRoot], + additionalRoots: [additionalRoot], }, }); - await fixture.expectPromptText(session.sessionId, "/skills", (text) => { - expect(text).toContain("Available skills:"); - expect(text).toContain("- session-root-skill: Session root skill"); - }); + await expectHelloSkillListed(fixture, session.sessionId); }); }); + +async function prepareHelloWorldAdditionalRoot(fixture: SpawnedAgentFixture): Promise { + const additionalRoot = path.join(fixture.workspaceDir, "..", "additional-roots", HELLO_WORLD_ADDITIONAL_ROOT_NAME); + fixture.writeSkill(HELLO_WORLD_SKILL, path.join(additionalRoot, ".agents", "skills")); + return additionalRoot; +} + +async function expectHelloSkillListed(fixture: SpawnedAgentFixture, sessionId: string): Promise { + await fixture.expectPromptText(sessionId, "/skills", (text) => { + expect(text).toContain("Available skills:"); + expect(text).toContain(`- ${HELLO_WORLD_SKILL.name}: ${HELLO_WORLD_SKILL.description}`); + }); +} diff --git a/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts b/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts index 7ec42d8..e740150 100644 --- a/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts +++ b/src/__tests__/CodexACPAgent/e2e/spawned-agent-fixture.ts @@ -4,6 +4,7 @@ import fs from "node:fs"; import path from "node:path"; import {Readable, Writable} from "node:stream"; import {expect, vi} from "vitest"; +import {CODEX_SKILL_FILE_NAME} from "../../../SkillDirectoryParser"; import {ModelId} from "../../../ModelId"; import {removeDirectoryWithRetry, writeCodexHomeConfig} from "../../acp-test-utils"; import type {PermissionResponder} from "./permission-responders"; @@ -17,10 +18,16 @@ export interface TestSkill { readonly body: string; } +export type CreateSessionOptions = { + readonly _meta?: Record | null; + readonly additionalDirectories?: string[]; + readonly mcpServers?: acp.McpServer[]; +}; + export interface SpawnedAgentFixture { readonly connection: acp.ClientSideConnection; readonly workspaceDir: string; - createSession(mcpServers?: acp.McpServer[]): Promise; + createSession(options?: CreateSessionOptions): Promise; restart(): Promise; writeSkill(skill: TestSkill, rootDir?: string): void; setPermissionResponder(responder: PermissionResponder): void; @@ -166,10 +173,13 @@ class SpawnedAgentFixtureImpl implements SpawnedAgentFixture { return this.paths.workspaceDir; } - async createSession(mcpServers: acp.McpServer[] = []): Promise { + async createSession(options?: CreateSessionOptions): Promise { + const sessionOptions = options ?? {}; return await this.connection.newSession({ cwd: this.workspaceDir, - mcpServers, + mcpServers: sessionOptions.mcpServers ?? [], + _meta: sessionOptions._meta ?? null, + additionalDirectories: sessionOptions.additionalDirectories ?? [], }); } @@ -183,7 +193,7 @@ class SpawnedAgentFixtureImpl implements SpawnedAgentFixture { const skillDirectory = path.join(skillsRoot, skill.name); fs.mkdirSync(skillDirectory, {recursive: true}); fs.writeFileSync( - path.join(skillDirectory, "SKILL.md"), + path.join(skillDirectory, CODEX_SKILL_FILE_NAME), [ "---", `name: ${skill.name}`,