diff --git a/apps/server/src/provider/providerExtensions.test.ts b/apps/server/src/provider/providerExtensions.test.ts index 02689aa96..afc68f543 100644 --- a/apps/server/src/provider/providerExtensions.test.ts +++ b/apps/server/src/provider/providerExtensions.test.ts @@ -31,6 +31,8 @@ import { derivePluginBackedSkillBundle, isCodexAppsDirectoryAccessDeniedError, mapCodexMcpServers, + mapCodexInstalledApps, + mergeCodexAppSources, mapCodexPluginInventory, mapCodexPluginDetail, parseClaudeMarketplaceManifest, @@ -43,6 +45,13 @@ import { readProviderInstructionFiles, readProviderExtensionsInventory, readProviderExtensionSkill, + createProviderExtensionSkill, + deleteProviderExtensionSkill, + setProviderExtensionSkillEnabled, + parseClaudeEnabledPlugins, + parseSkillFrontMatter, + skillDirectoryUnderRoots, + annotateClaudeSkillCapabilities, refreshProviderExtensionPluginMarketplaces, startProviderExtensionMcpOAuth, writeInstructionFile, @@ -259,6 +268,7 @@ const codexInventoryPeerHandlers: Record unknown) "skills/list": () => ({ data: [] }), "mcpServerStatus/list": () => ({ data: [] }), "app/list": () => ({ data: [] }), + "app/installed": () => ({ apps: [] }), }; function makeSettings(overrides: Record = {}): ServerSettingsContract { @@ -280,9 +290,26 @@ function claudeInventoryProcessFor(args: ReadonlyArray) { if (args[0] === "mcp" && args[1] === "list") { return makeProcessResult(""); } + if (args[0] === "plugin" && (args[1] === "enable" || args[1] === "disable")) { + return makeProcessResult(`Successfully ${args[1]}d plugin: ${args[2] ?? ""}`); + } return makeProcessResult("", `unexpected claude command: ${args.join(" ")}`, 1); } +/** A Claude-only settings object rooted at a throwaway home, which every skill test needs. */ +function makeClaudeSkillSettings(claudeHome: string): ServerSettingsContract { + return makeSettings({ + providers: { + codex: { enabled: false }, + claudeAgent: { enabled: true, binaryPath: "claude", homePath: claudeHome }, + cursor: { enabled: false }, + opencode: { enabled: false }, + }, + }); +} + +const CLAUDE_SKILL_PROVIDER = ProviderInstanceId.make("claudeAgent"); + describe("provider extensions inventory", () => { it("treats Codex app-directory 403s as optional inventory misses", () => { assert.equal( @@ -1222,6 +1249,109 @@ Per-component (rounded) assert.equal(codexSkillConfigWriteParams({ enabled: true }), null); }); + it.effect("toggles every same-name Codex copy the deduped row stands for", () => { + const userPath = "/home/.codex/skills/imagegen/SKILL.md"; + const systemPath = "/home/.codex/skills/.system/imagegen/SKILL.md"; + const repoPath = "/repo/.codex/skills/imagegen/SKILL.md"; + const writes: Array = []; + const peer = makeCodexAppServerPeer({ + ...codexInventoryPeerHandlers, + "skills/list": () => ({ + data: [ + { + cwd: process.cwd(), + errors: [], + skills: [ + { + name: "imagegen", + path: userPath, + scope: "user", + description: "Personal copy.", + enabled: true, + }, + // Same name, shipped by Codex: one row in the UI, so one toggle governs both. + { + name: "ImageGen", + path: systemPath, + scope: "system", + description: "Built-in copy.", + enabled: true, + }, + // A project skill that happens to share the name stays independent. + { + name: "imagegen", + path: repoPath, + scope: "repo", + description: "Project copy.", + enabled: true, + }, + { + name: "openai-docs", + path: "/home/.codex/skills/.system/openai-docs/SKILL.md", + scope: "system", + description: "Unrelated.", + enabled: true, + }, + ], + }, + ], + }), + "skills/config/write": (params) => { + writes.push(params); + return { effectiveEnabled: false }; + }, + }); + const spawnerLayer = Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, peer.spawner); + + return Effect.gen(function* () { + const result = yield* setProviderExtensionSkillEnabled({ + request: { + cwd: process.cwd(), + providerInstanceId: ProviderInstanceId.make("codex"), + path: userPath, + enabled: false, + }, + settings: makeSettings(), + }); + + assert.deepEqual(result, { effectiveEnabled: false }); + assert.deepEqual(writes, [ + { enabled: false, path: userPath }, + { enabled: false, path: systemPath }, + ]); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it.effect("still toggles the requested Codex skill when the listing fails", () => { + const userPath = "/home/.codex/skills/imagegen/SKILL.md"; + const writes: Array = []; + // No `skills/list` handler, so the peer answers it with a JSON-RPC error. + const peer = makeCodexAppServerPeer({ + initialize: codexInventoryPeerHandlers.initialize!, + "skills/config/write": (params) => { + writes.push(params); + return { effectiveEnabled: false }; + }, + }); + const spawnerLayer = Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, peer.spawner); + + return Effect.gen(function* () { + const result = yield* setProviderExtensionSkillEnabled({ + request: { + cwd: process.cwd(), + providerInstanceId: ProviderInstanceId.make("codex"), + path: userPath, + enabled: false, + }, + settings: makeSettings(), + }); + + // A partial toggle beats a failed one. + assert.deepEqual(result, { effectiveEnabled: false }); + assert.deepEqual(writes, [{ enabled: false, path: userPath }]); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + it.effect( "returns a Codex provider error instead of hanging when app-server never responds", () => { @@ -1271,10 +1401,13 @@ Per-component (rounded) assert.equal(codex?.status, "ready"); assert.equal(codex?.mcpServersStatus, "deferred"); - assert.equal(codex?.appsStatus, "deferred"); + // The connected snapshot is cheap and always read; only the directory stays deferred. + assert.equal(codex?.appsStatus, "ready"); + assert.equal(codex?.appsCatalogStatus, "deferred"); assert.equal(peer.calls.includes("plugin/list"), true); assert.equal(peer.calls.includes("skills/list"), true); assert.equal(peer.calls.includes("mcpServerStatus/list"), false); + assert.equal(peer.calls.includes("app/installed"), true); assert.equal(peer.calls.includes("app/list"), false); }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); }); @@ -1307,7 +1440,9 @@ Per-component (rounded) ); assert.equal(codex?.status, "ready"); - assert.equal(codex?.appsStatus, "error"); + // The catalog timed out; the connected snapshot still answered, so the section is fine. + assert.equal(codex?.appsStatus, "ready"); + assert.equal(codex?.appsCatalogStatus, "error"); assert.equal(codex?.appsMessage, "Timed out reading Codex apps."); assert.deepEqual(codex?.apps, []); assert.equal(codex?.mcpServersStatus, "deferred"); @@ -1315,6 +1450,129 @@ Per-component (rounded) }, ); + it.effect("reports connected apps from the local snapshot enriched by app/read", () => { + const peer = makeCodexAppServerPeer({ + ...codexInventoryPeerHandlers, + "app/installed": () => ({ + apps: [ + { id: "connector-1", enabled: true, callable: true, runtimeName: "alpaca-runtime" }, + { id: "connector-2", enabled: false, callable: false }, + ], + }), + "app/read": () => ({ + apps: [ + { + id: "connector-1", + name: "Alpaca", + description: "Trade from chat.", + iconUrl: "https://example.test/alpaca.png", + }, + ], + missingAppIds: ["connector-2"], + }), + // The catalog stays deferred, so the connected list is entirely snapshot-driven. + "app/list": "never", + }); + const spawnerLayer = Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, peer.spawner); + + return Effect.gen(function* () { + invalidateCodexAppsCache(); + const result = yield* readProviderExtensionsInventory({ + request: { cwd: process.cwd(), includeMcpServers: false, includeApps: false }, + settings: makeSettings(), + providers: [], + }); + const codex = result.providers.find( + (provider) => provider.instanceId === ProviderInstanceId.make("codex"), + ); + + assert.equal(codex?.appsStatus, "ready"); + assert.equal(codex?.appsCatalogStatus, "deferred"); + assert.deepEqual( + codex?.apps.map((app) => app.name), + ["Alpaca", "connector-2"], + ); + const enriched = codex?.apps.find((app) => app.id === "connector-1"); + assert.equal(enriched?.description, "Trade from chat."); + assert.equal(enriched?.iconUrl, "https://example.test/alpaca.png"); + // Everything in the snapshot is connected, whatever app/read knew about it. + assert.equal(enriched?.accessible, true); + const unread = codex?.apps.find((app) => app.id === "connector-2"); + assert.equal(unread?.accessible, true); + assert.equal(unread?.enabled, false); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it.effect("keeps the connected apps when app/read fails", () => { + const peer = makeCodexAppServerPeer({ + ...codexInventoryPeerHandlers, + "app/installed": () => ({ + apps: [{ id: "connector-1", enabled: true, callable: true, runtimeName: "Alpaca" }], + }), + "app/list": "never", + }); + const spawnerLayer = Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, peer.spawner); + + return Effect.gen(function* () { + invalidateCodexAppsCache(); + const result = yield* readProviderExtensionsInventory({ + request: { cwd: process.cwd(), includeMcpServers: false, includeApps: false }, + settings: makeSettings(), + providers: [], + }); + const codex = result.providers.find( + (provider) => provider.instanceId === ProviderInstanceId.make("codex"), + ); + + // `app/read` is unhandled by this peer, so the bare snapshot has to stand on its own. + assert.equal(codex?.appsStatus, "ready"); + assert.deepEqual( + codex?.apps.map((app) => app.name), + ["Alpaca"], + ); + assert.equal(codex?.apps[0]?.iconUrl, undefined); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it("falls back to the app id when the snapshot carries no runtime name", () => { + const apps = mapCodexInstalledApps({ + apps: [{ id: "connector-9", enabled: true, callable: false }], + }); + + assert.deepEqual(apps, [ + { id: "connector-9", name: "connector-9", enabled: true, accessible: true }, + ]); + }); + + it("lets directory metadata fill gaps the connected snapshot cannot", () => { + const merged = mergeCodexAppSources( + [{ id: "connector-1", name: "connector-1", enabled: true, accessible: true }], + [ + { + id: "connector-1", + name: "Alpaca", + description: "Trade from chat.", + iconUrl: "https://example.test/alpaca.png", + accessible: false, + }, + { id: "connector-2", name: "Bravo", accessible: false }, + ], + ); + + assert.deepEqual( + merged.map((app) => app.name), + ["Alpaca", "Bravo"], + ); + const connected = merged.find((app) => app.id === "connector-1"); + // A snapshot name that fell back to the id must not overwrite the directory's real name. + assert.equal(connected?.name, "Alpaca"); + assert.equal(connected?.description, "Trade from chat."); + assert.equal(connected?.iconUrl, "https://example.test/alpaca.png"); + // Only the snapshot decides connectivity, so the directory's stale flag loses. + assert.equal(connected?.accessible, true); + assert.equal(merged.find((app) => app.id === "connector-2")?.accessible, false); + }); + it.effect("serves Codex apps from cache without re-issuing the slow app/list request", () => { const appListHandlers = { ...codexInventoryPeerHandlers, @@ -1581,6 +1839,360 @@ Per-component (rounded) }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); }); + it("reads folded and literal block scalars in skill front matter", () => { + // Shape taken from a real plugin-bundled skill, which rendered as just ">" before this. + const folded = parseSkillFrontMatter( + [ + "name: analyzing-expensive-users", + "description: >", + " Analyze the most expensive users in AI observability and explain why they cost so much.", + " Use when the user asks about top spenders, expensive users, per-user LLM cost.", + "allowed-tools: Read", + ].join("\n"), + ); + assert.equal( + folded.get("description"), + "Analyze the most expensive users in AI observability and explain why they cost so much. Use when the user asks about top spenders, expensive users, per-user LLM cost.", + ); + // The key after the block must still be read, not swallowed by it. + assert.equal(folded.get("allowedtools"), "Read"); + + const literal = parseSkillFrontMatter( + ["description: |-", " First line.", " Second line.", "name: keeper"].join("\n"), + ); + assert.equal(literal.get("description"), "First line.\nSecond line."); + assert.equal(literal.get("name"), "keeper"); + + // Plain scalars keep working, quotes and all. + assert.equal( + parseSkillFrontMatter('description: "Trade: from chat."').get("description"), + "Trade: from chat.", + ); + }); + + it("reads the enabled state Claude writes for skills-dir plugins", () => { + const enabled = parseClaudeEnabledPlugins( + '{"enabledPlugins":{"note-taker@skills-dir":false,"other@marketplace":true,"bad":1}}', + ); + + assert.equal(enabled.get("note-taker@skills-dir"), false); + assert.equal(enabled.get("other@marketplace"), true); + // A non-boolean entry says nothing about the skill, so it must not read as disabled. + assert.equal(enabled.has("bad"), false); + assert.equal(parseClaudeEnabledPlugins("not json").size, 0); + }); + + it.effect("treats only a direct child of a skills root as managed", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const root = path.join("/home", ".claude", "skills"); + + assert.equal( + skillDirectoryUnderRoots(path, [root], path.join(root, "writer", "SKILL.md")), + path.join(root, "writer"), + ); + // Namespaced skills sit a level deeper; deleting one would leave its namespace behind. + assert.equal( + skillDirectoryUnderRoots(path, [root], path.join(root, "team", "writer", "SKILL.md")), + null, + ); + assert.equal( + skillDirectoryUnderRoots(path, [root], path.join("/elsewhere", "writer", "SKILL.md")), + null, + ); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("leaves a plugin-bundled skill without a toggle or a delete", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const userSkillsRoot = path.join("/home", ".claude", "skills"); + const [bundled, personal] = annotateClaudeSkillCapabilities( + [ + { + name: "bundled", + path: path.join(userSkillsRoot, "bundled", "SKILL.md"), + bundleId: "helper@market", + }, + { name: "personal", path: path.join(userSkillsRoot, "personal", "SKILL.md") }, + ], + { + path, + userSkillsRoot, + writableRoots: [userSkillsRoot], + enabledPlugins: new Map(), + }, + ); + + // A bundled skill follows its plugin even when it was found in a writable root. + assert.equal(bundled?.canToggle, undefined); + assert.equal(bundled?.canDelete, undefined); + assert.equal(personal?.canToggle, true); + assert.equal(personal?.canDelete, true); + assert.equal(personal?.enabled, true); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("creates a skill the inventory then reports as toggleable and deletable", () => { + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => + Effect.succeed( + claudeInventoryProcessFor( + (command as unknown as { readonly args: ReadonlyArray }).args, + ), + ), + ), + ); + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-create-repo-", + }); + const claudeHome = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-create-home-", + }); + const settings = makeClaudeSkillSettings(claudeHome); + + const created = yield* createProviderExtensionSkill({ + request: { + cwd, + providerInstanceId: CLAUDE_SKILL_PROVIDER, + name: "release-notes", + description: "Write the release notes: carefully.", + }, + settings, + }); + + assert.equal( + created.path, + path.join(claudeHome, ".claude", "skills", "release-notes", "SKILL.md"), + ); + const contents = yield* fileSystem.readFileString(created.path); + assert.include(contents, "name: release-notes"); + // A description with a colon in it must not break the YAML front matter. + assert.include(contents, 'description: "Write the release notes: carefully."'); + + // Claude reports a disabled skills-dir skill only through its settings.json map. + yield* fileSystem.writeFileString( + path.join(claudeHome, ".claude", "settings.json"), + '{"enabledPlugins":{"release-notes@skills-dir":false}}', + ); + + const inventory = yield* readProviderExtensionsInventory({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER }, + settings, + providers: [], + }); + const skill = inventory.providers + .flatMap((provider) => provider.skills) + .find((entry) => entry.path === created.path); + + assert.equal(skill?.canToggle, true); + assert.equal(skill?.canDelete, true); + assert.equal(skill?.enabled, false); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it.effect("refuses a skill name that is not kebab-case or already taken", () => { + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => Effect.succeed(makeProcessResult(""))), + ); + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-name-repo-", + }); + const claudeHome = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-name-home-", + }); + const settings = makeClaudeSkillSettings(claudeHome); + + const invalid = yield* createProviderExtensionSkill({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER, name: "Release Notes" }, + settings, + }).pipe(Effect.flip); + assert.include(invalid.message, "lower-case letters"); + + yield* fileSystem.makeDirectory(path.join(claudeHome, ".claude", "skills", "taken"), { + recursive: true, + }); + const duplicate = yield* createProviderExtensionSkill({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER, name: "taken" }, + settings, + }).pipe(Effect.flip); + assert.include(duplicate.message, "already exists"); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it.effect("deletes a personal skill folder and refuses everything outside one", () => { + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => + Effect.succeed( + claudeInventoryProcessFor( + (command as unknown as { readonly args: ReadonlyArray }).args, + ), + ), + ), + ); + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-delete-repo-", + }); + const claudeHome = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-delete-home-", + }); + const skillsRoot = path.join(claudeHome, ".claude", "skills"); + const settings = makeClaudeSkillSettings(claudeHome); + + const doomedDirectory = path.join(skillsRoot, "doomed"); + const doomedPath = path.join(doomedDirectory, "SKILL.md"); + yield* fileSystem.makeDirectory(doomedDirectory, { recursive: true }); + yield* fileSystem.writeFileString(doomedPath, "---\nname: doomed\n---\n"); + // A namespaced skill is reported by the inventory but is not a direct child of the root. + const nestedDirectory = path.join(skillsRoot, "team", "nested"); + const nestedPath = path.join(nestedDirectory, "SKILL.md"); + yield* fileSystem.makeDirectory(nestedDirectory, { recursive: true }); + yield* fileSystem.writeFileString(nestedPath, "---\nname: nested\n---\n"); + const outsidePath = path.join(cwd, "not-a-skill.md"); + yield* fileSystem.writeFileString(outsidePath, "Leave this alone.\n"); + + yield* readProviderExtensionsInventory({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER }, + settings, + providers: [], + }); + + const outside = yield* deleteProviderExtensionSkill({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER, path: outsidePath }, + settings, + }).pipe(Effect.flip); + assert.equal(outside.message, "Skill is not in the current provider extensions inventory."); + + const nested = yield* deleteProviderExtensionSkill({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER, path: nestedPath }, + settings, + }).pipe(Effect.flip); + assert.equal(nested.message, "Skill is not in a skills folder Threadlines manages."); + assert.equal(yield* fileSystem.exists(nestedPath), true); + + const deleted = yield* deleteProviderExtensionSkill({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER, path: doomedPath }, + settings, + }); + assert.deepEqual(deleted, { deleted: true }); + assert.equal(yield* fileSystem.exists(doomedDirectory), false); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it.effect("refuses to delete a skill entry that is a symlink", () => { + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => + Effect.succeed( + claudeInventoryProcessFor( + (command as unknown as { readonly args: ReadonlyArray }).args, + ), + ), + ), + ); + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-symlink-repo-", + }); + const claudeHome = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-symlink-home-", + }); + const linkedDirectory = path.join(claudeHome, ".claude", "skills", "linked"); + const linkedPath = path.join(linkedDirectory, "SKILL.md"); + const target = path.join(cwd, "real-skill.md"); + yield* fileSystem.writeFileString(target, "---\nname: linked\n---\n"); + yield* fileSystem.makeDirectory(linkedDirectory, { recursive: true }); + yield* fileSystem.symlink(target, linkedPath); + const settings = makeClaudeSkillSettings(claudeHome); + + yield* readProviderExtensionsInventory({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER }, + settings, + providers: [], + }); + + const error = yield* deleteProviderExtensionSkill({ + request: { cwd, providerInstanceId: CLAUDE_SKILL_PROVIDER, path: linkedPath }, + settings, + }).pipe(Effect.flip); + + assert.include(error.message, "not a regular file"); + assert.equal(yield* fileSystem.exists(target), true); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + + it.effect("toggles a Claude user skill through its skills-dir plugin id", () => { + const calls: Array> = []; + const spawnerLayer = Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make((command) => { + const args = (command as unknown as { readonly args: ReadonlyArray }).args; + calls.push(args); + return Effect.succeed(claudeInventoryProcessFor(args)); + }), + ); + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const cwd = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-toggle-repo-", + }); + const claudeHome = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-skill-toggle-home-", + }); + const skillPath = path.join(claudeHome, ".claude", "skills", "note-taker", "SKILL.md"); + yield* fileSystem.makeDirectory(path.dirname(skillPath), { recursive: true }); + yield* fileSystem.writeFileString(skillPath, "---\nname: note-taker\n---\n"); + const settings = makeClaudeSkillSettings(claudeHome); + + const result = yield* setProviderExtensionSkillEnabled({ + request: { + cwd, + providerInstanceId: CLAUDE_SKILL_PROVIDER, + path: skillPath, + enabled: false, + }, + settings, + }); + + assert.deepEqual(result, { effectiveEnabled: false }); + assert.deepEqual(calls.at(-1), ["plugin", "disable", "note-taker@skills-dir"]); + + // A project skill has no skills-dir plugin behind it, so it must be refused rather than + // guessed at. + const projectSkillPath = path.join(cwd, ".claude", "skills", "local", "SKILL.md"); + const error = yield* setProviderExtensionSkillEnabled({ + request: { + cwd, + providerInstanceId: CLAUDE_SKILL_PROVIDER, + path: projectSkillPath, + enabled: false, + }, + settings, + }).pipe(Effect.flip); + assert.include(error.message, "personal skills folder"); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + it.effect("discovers Claude skills from user, ancestor, current, and nested roots", () => { const spawner = ChildProcessSpawner.make((command) => { const childProcess = command as unknown as { @@ -1693,6 +2305,54 @@ Per-component (rounded) }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); }); + it.effect("keeps a personal skill user-scoped when the project lives under the home", () => { + const spawner = ChildProcessSpawner.make((command) => { + const childProcess = command as unknown as { + readonly args: ReadonlyArray; + }; + return Effect.succeed(claudeInventoryProcessFor(childProcess.args)); + }); + const spawnerLayer = Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, spawner); + + return Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + // Real projects sit under the home directory, so the ancestor walk reaches + // /.claude/skills and must not re-file personal skills as project skills. + const claudeHome = yield* fileSystem.makeTempDirectoryScoped({ + prefix: "threadlines-claude-skills-nested-home-", + }); + const cwd = path.join(claudeHome, "projects", "app"); + yield* fileSystem.makeDirectory(cwd, { recursive: true }); + const skillPath = path.join(claudeHome, ".claude", "skills", "personal", "SKILL.md"); + yield* fileSystem.makeDirectory(path.dirname(skillPath), { recursive: true }); + yield* fileSystem.writeFileString( + skillPath, + ["---", "name: personal", "---", "A personal skill."].join("\n"), + ); + + const result = yield* readProviderExtensionsInventory({ + request: { + cwd, + providerInstanceId: ProviderInstanceId.make("claudeAgent"), + }, + settings: makeSettings({ + providers: { + codex: { enabled: false }, + claudeAgent: { enabled: true, binaryPath: "claude", homePath: claudeHome }, + cursor: { enabled: false }, + opencode: { enabled: false }, + }, + }), + providers: [], + }); + const skill = result.providers[0]?.skills.find((entry) => entry.name === "personal"); + + assert.equal(skill?.scope, "user"); + assert.equal(skill?.source, "Claude user"); + }).pipe(Effect.provide(Layer.mergeAll(NodeServices.layer, spawnerLayer))); + }); + it.effect("starts Claude MCP login through the configured Claude CLI", () => { const ptyCalls: Array<{ readonly shell: string; readonly args: ReadonlyArray }> = []; const calls: Array<{ diff --git a/apps/server/src/provider/providerExtensions.ts b/apps/server/src/provider/providerExtensions.ts index a671814ad..83ed9c80f 100644 --- a/apps/server/src/provider/providerExtensions.ts +++ b/apps/server/src/provider/providerExtensions.ts @@ -61,6 +61,10 @@ import { type ProviderExtensionsInventoryResult, type ProviderExtensionProviderInventory, type ProviderExtensionSkill, + type ProviderExtensionSkillCreateInput, + type ProviderExtensionSkillCreateResult, + type ProviderExtensionSkillDeleteInput, + type ProviderExtensionSkillDeleteResult, type ProviderExtensionSkillReadInput, type ProviderExtensionSkillReadResult, type ProviderExtensionSkillToggleInput, @@ -79,8 +83,12 @@ import { } from "@threadlines/contracts"; import { codexAppServerCommandOptions } from "./codexAppServerArgs.ts"; -import { makeClaudeEnvironment } from "./Drivers/ClaudeHome.ts"; -import { materializeCodexShadowHome, resolveCodexHomeLayout } from "./Drivers/CodexHomeLayout.ts"; +import { makeClaudeEnvironment, resolveClaudeHomePath } from "./Drivers/ClaudeHome.ts"; +import { + materializeCodexShadowHome, + resolveCodexHomeLayout, + type CodexHomeLayout, +} from "./Drivers/CodexHomeLayout.ts"; import { buildCodexInitializeParams } from "./Layers/CodexProvider.ts"; import { deriveProviderInstanceConfigMap } from "./Layers/ProviderInstanceRegistryHydration.ts"; import { mergeProviderInstanceEnvironment } from "./ProviderInstanceEnvironment.ts"; @@ -109,6 +117,8 @@ const CODEX_MCP_ORIGIN_TIMEOUT = Duration.seconds(3); const CODEX_APP_SERVER_ACTION_TIMEOUT = Duration.seconds(120); const CLAUDE_PLUGIN_ACTION_TIMEOUT = Duration.seconds(120); const CODEX_EXTENSION_INVENTORY_PAGE_LIMIT = 100; +/** `app/read` accepts at most this many ids per call. */ +const CODEX_APP_READ_MAX_IDS = 100; const MAX_CODEX_MCP_ORIGIN_PLUGINS = 40; const CODEX_MCP_OAUTH_DEFAULT_TIMEOUT_SECONDS = 300; const CODEX_MCP_OAUTH_MAX_TIMEOUT_SECONDS = 900; @@ -785,6 +795,19 @@ export function codexMarketplaceLoadErrorMessage( ); } +/** + * User skills live in `/skills`. With a shadow home in play the app-server reports + * paths through the shadow, which symlinks that directory back to the shared home, so both spellings + * count as the same root. + */ +export function codexUserSkillsRoots(path: Path.Path, layout: CodexHomeLayout): string[] { + const roots = [path.join(layout.sharedHomePath, "skills")]; + if (layout.effectiveHomePath && layout.effectiveHomePath !== layout.sharedHomePath) { + roots.push(path.join(layout.effectiveHomePath, "skills")); + } + return roots; +} + function mapCodexSkills( response: CodexSchema.V2SkillsListResponse, cwd: string, @@ -816,6 +839,41 @@ function mapCodexSkills( .toSorted((left, right) => left.name.localeCompare(right.name)); } +/** Scopes whose entries a name-matched toggle is allowed to touch. */ +const CODEX_SHADOWABLE_SKILL_SCOPES = new Set(["user", "system"]); + +/** + * Codex keys a skill's enabled state by path, so a personal copy and the built-in it shadows carry + * two independent flags under one name. The list shows them as a single row, so its toggle has to + * govern both: writing only one leaves the other live and the row lying about it. + * + * Project-scoped (`repo`) and plugin-bundled entries never participate. Two projects using the + * same skill name are genuinely unrelated. + */ +export function codexShadowedSkillWritePaths( + response: CodexSchema.V2SkillsListResponse, + target: { readonly name?: string | undefined; readonly path?: string | undefined }, +): string[] { + const entries = response.data.flatMap((entry) => entry.skills); + const nameKey = (value: string | null | undefined) => optionalText(value)?.toLowerCase(); + const targetPath = optionalText(target.path); + const targetName = + nameKey(target.name) ?? + nameKey(entries.find((entry) => requiredText(entry.path) === targetPath)?.name); + if (!targetName) return []; + + const paths = new Set(); + for (const entry of entries) { + if (!CODEX_SHADOWABLE_SKILL_SCOPES.has(entry.scope)) continue; + if (nameKey(entry.name) !== targetName) continue; + const path = requiredText(entry.path); + // The requested path is written on its own; this is only the copies alongside it. + if (!path || path === targetPath) continue; + paths.add(path); + } + return [...paths]; +} + function codexMcpAuthStatusLabel( authStatus: CodexSchema.V2ListMcpServerStatusResponse__McpAuthStatus, ): string { @@ -983,12 +1041,18 @@ const readCodexMcpPluginOwners = Effect.fn("providerExtensions.readCodexMcpPlugi }, ); +function compareAppsByName(left: ProviderExtensionApp, right: ProviderExtensionApp): number { + return left.name.localeCompare(right.name); +} + function mapCodexApps(response: CodexSchema.V2AppsListResponse): ProviderExtensionApp[] { return response.data .flatMap((app) => { const id = requiredText(app.id); const name = requiredText(app.name); if (!id || !name) return []; + const iconUrl = optionalText(app.logoUrl ?? null); + const iconUrlDark = optionalText(app.logoUrlDark ?? null); return [ { id, @@ -996,10 +1060,99 @@ function mapCodexApps(response: CodexSchema.V2AppsListResponse): ProviderExtensi description: optionalText(app.description ?? app.appMetadata?.seoDescription ?? null), enabled: app.isEnabled, accessible: app.isAccessible, + ...(iconUrl ? { iconUrl } : {}), + ...(iconUrlDark ? { iconUrlDark } : {}), }, ]; }) - .toSorted((left, right) => left.name.localeCompare(right.name)); + .toSorted(compareAppsByName); +} + +/** + * `app/installed` reads a locally committed runtime snapshot, so it answers without a backend + * round-trip. It carries connectivity but only a best-effort name, which `app/read` then fills in. + */ +export function mapCodexInstalledApps( + response: CodexSchema.V2AppsInstalledResponse, +): ProviderExtensionApp[] { + return response.apps + .flatMap((app) => { + const id = requiredText(app.id); + if (!id) return []; + return [ + { + id, + name: optionalText(app.runtimeName ?? null) ?? id, + enabled: app.enabled, + // Being in the snapshot is what "connected" means; the directory says nothing about it. + accessible: true, + }, + ]; + }) + .toSorted(compareAppsByName); +} + +/** Canonical names, descriptions, and logos for apps already known to be installed. */ +export function mergeCodexAppMetadata( + apps: ReadonlyArray, + response: CodexSchema.V2AppsReadResponse, +): ProviderExtensionApp[] { + const metadataById = new Map( + response.apps.flatMap((entry) => { + const id = requiredText(entry.id); + return id ? [[id, entry] as const] : []; + }), + ); + return apps + .map((app) => { + const metadata = metadataById.get(app.id); + if (!metadata) return app; + const description = optionalText(metadata.description ?? null); + const iconUrl = optionalText(metadata.iconUrl ?? null); + const iconUrlDark = optionalText(metadata.iconUrlDark ?? null); + return { + ...app, + name: requiredText(metadata.name) ?? app.name, + ...(description ? { description } : {}), + ...(iconUrl ? { iconUrl } : {}), + ...(iconUrlDark ? { iconUrlDark } : {}), + }; + }) + .toSorted(compareAppsByName); +} + +/** + * The connected snapshot and the full directory describe the same apps from different angles. + * Connectivity is only the snapshot's to state; everything descriptive takes whichever source + * actually has a value. + */ +export function mergeCodexAppSources( + installed: ReadonlyArray, + catalog: ReadonlyArray, +): ProviderExtensionApp[] { + const merged = new Map(); + for (const app of catalog) merged.set(app.id, app); + for (const app of installed) { + const listed = merged.get(app.id); + if (!listed) { + merged.set(app.id, app); + continue; + } + // A snapshot name that fell back to the id carries no information, so the directory wins. + const name = app.name === app.id ? listed.name : app.name; + const description = app.description ?? listed.description; + const iconUrl = app.iconUrl ?? listed.iconUrl; + const iconUrlDark = app.iconUrlDark ?? listed.iconUrlDark; + merged.set(app.id, { + ...listed, + ...app, + name, + ...(description ? { description } : {}), + ...(iconUrl ? { iconUrl } : {}), + ...(iconUrlDark ? { iconUrlDark } : {}), + }); + } + return [...merged.values()].toSorted(compareAppsByName); } /** @@ -1358,6 +1511,7 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe | "mcpServersTruncated" | "apps" | "appsStatus" + | "appsCatalogStatus" | "appsMessage" | "appsTruncated" | "status" @@ -1366,6 +1520,7 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe never, FileSystem.FileSystem | Path.Path | ChildProcessSpawner.ChildProcessSpawner > { + const path = yield* Path.Path; const layout = yield* resolveCodexHomeLayout(input.config); const materialized = yield* materializeCodexShadowHome(layout).pipe(Effect.result); if (Result.isFailure(materialized)) { @@ -1429,6 +1584,8 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe const appListWithoutThreadParams = { limit: CODEX_EXTENSION_INVENTORY_PAGE_LIMIT, }; + const appInstalledParams: CodexSchema.V2AppsInstalledParams = + input.providerThreadId !== undefined ? { threadId: input.providerThreadId } : {}; const emptyMcpServerResponse: CodexSchema.V2ListMcpServerStatusResponse = { data: [], }; @@ -1445,6 +1602,32 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe ) : Effect.succeed(Result.succeed(emptyMcpServerResponse)); + // The connected set is a local snapshot read, so it runs on every inventory load and the + // Apps section can render immediately. The directory behind `app/list` stays deferred. + const installedAppsEffect = client.request("app/installed", appInstalledParams).pipe( + Effect.catch((cause) => + input.providerThreadId !== undefined && isThreadNotFoundError(cause) + ? client.request("app/installed", {}) + : Effect.fail(cause), + ), + Effect.flatMap((snapshot) => { + const base = mapCodexInstalledApps(snapshot); + if (base.length === 0) return Effect.succeed(base); + return client + .request("app/read", { + appIds: base.slice(0, CODEX_APP_READ_MAX_IDS).map((app) => app.id), + includeTools: false, + }) + .pipe( + // Names and logos are a nicety. Losing them leaves the connected list intact, + // which beats reporting the whole section as failed. + Effect.map((metadata) => mergeCodexAppMetadata(base, metadata)), + Effect.catch(() => Effect.succeed(base)), + ); + }), + collectCodexRequest("connected apps"), + ); + const appsEffect = fetchApps ? client.request("app/list", appListParams).pipe( Effect.catch((cause) => @@ -1515,8 +1698,8 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe ).pipe(Effect.forkScoped); // The requests are independent JSON-RPC calls; serializing them just delays whichever - // runs last and starts its timeout window late, so let all four go at once. - const [plugins, skills, mcpServerResponse, apps] = yield* Effect.all( + // runs last and starts its timeout window late, so let them all go at once. + const [plugins, skills, mcpServerResponse, apps, installedApps] = yield* Effect.all( [ pluginsEffect, client.request("skills/list", { cwds: [input.cwd] }).pipe( @@ -1525,8 +1708,9 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe ), mcpServersEffect, appsEffect, + installedAppsEffect, ], - { concurrency: 4 }, + { concurrency: 5 }, ); const mcpPluginOwnersExit = mcpPluginOwnersFiber.pollUnsafe(); const mcpPluginOwners = @@ -1536,7 +1720,7 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe const mcpServers = Result.isFailure(mcpServerResponse) ? Result.fail(mcpServerResponse.failure) : Result.succeed(mapCodexMcpServers(mcpServerResponse.success, mcpPluginOwners)); - return { includeMcpServers, plugins, skills, mcpServers, apps }; + return { includeMcpServers, plugins, skills, mcpServers, apps, installedApps }; }), ).pipe( Effect.result, @@ -1580,25 +1764,44 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe const mcpServersMessage = Result.isFailure(data.mcpServers) ? resultMessage(data.mcpServers) : undefined; - // Apps failures report through their own section like MCP servers do: the Apps tab shows the - // error and a retry, without flagging the whole provider as "loaded with issues". When a + // Apps failures report through their own section like MCP servers do: the Apps section shows + // the error and a retry, without flagging the whole provider as "loaded with issues". When a // refresh fails but an earlier read succeeded, the stale list beats an empty error state. const appsFailureMessage = Result.isFailure(data.apps) ? resultMessage(data.apps) : undefined; const staleAppsFallback = Result.isFailure(data.apps) && cachedApps !== undefined ? cachedApps.apps : undefined; - const apps = Result.isSuccess(data.apps) ? data.apps.success : (staleAppsFallback ?? []); - const appsLoaded = Result.isSuccess(data.apps) || staleAppsFallback !== undefined; - const appsStatus = - includeApps || freshCachedApps !== undefined ? (appsLoaded ? "ready" : "error") : "deferred"; + const catalogApps = Result.isSuccess(data.apps) ? data.apps.success : (staleAppsFallback ?? []); + const catalogLoaded = Result.isSuccess(data.apps) || staleAppsFallback !== undefined; + const installedApps = Result.isSuccess(data.installedApps) ? data.installedApps.success : []; + const apps = mergeCodexAppSources(installedApps, catalogApps); + const installedAppsFailureMessage = Result.isFailure(data.installedApps) + ? resultMessage(data.installedApps) + : undefined; + // The connected list is what the section renders, so its own read decides the section status. + const appsStatus = Result.isSuccess(data.installedApps) ? "ready" : "error"; + const appsCatalogStatus = + includeApps || freshCachedApps !== undefined + ? catalogLoaded + ? "ready" + : "error" + : "deferred"; const appsMessage = - appsFailureMessage !== undefined && staleAppsFallback !== undefined - ? `${appsFailureMessage} Showing the last loaded apps.` - : appsFailureMessage; + [ + installedAppsFailureMessage, + appsFailureMessage !== undefined && staleAppsFallback !== undefined + ? `${appsFailureMessage} Showing the last loaded apps.` + : appsFailureMessage, + ] + .filter((message): message is string => Boolean(message)) + .join(" ") || undefined; const plugins = Result.isSuccess(data.plugins) ? data.plugins.success.plugins : []; const marketplaces = Result.isSuccess(data.plugins) ? data.plugins.success.marketplaces : []; const skills = Result.isSuccess(data.skills) - ? annotatePluginBackedSkills(data.skills.success, plugins) + ? annotateCodexSkillCapabilities(annotatePluginBackedSkills(data.skills.success, plugins), { + path, + userSkillsRoots: codexUserSkillsRoots(path, layout), + }) : []; const mcpServersStatus = data.includeMcpServers ? Result.isSuccess(data.mcpServers) @@ -1621,8 +1824,11 @@ const readCodexAppServerInventory = Effect.fn("providerExtensions.readCodexAppSe : {}), apps, appsStatus, + appsCatalogStatus, ...(appsMessage ? { appsMessage } : {}), - ...(apps.length === CODEX_EXTENSION_INVENTORY_PAGE_LIMIT ? { appsTruncated: true } : {}), + ...(catalogApps.length === CODEX_EXTENSION_INVENTORY_PAGE_LIMIT + ? { appsTruncated: true } + : {}), }; }, ); @@ -2364,20 +2570,52 @@ const waitForClaudeMcpLoginStatus = Effect.fn("providerExtensions.waitForClaudeM }, ); +/** The block scalar openers worth supporting: plugin authors use these for long descriptions. */ +const SKILL_BLOCK_SCALAR_MARKERS = new Set([">", ">-", "|", "|-"]); + +/** + * Front matter is read by hand rather than with a YAML library, because these files only ever use + * top-level `key: value` plus block scalars. A folded block (`>`) joins its lines with spaces, a + * literal block (`|`) keeps the newlines. Without this a long description reads as just ">". + */ +export function parseSkillFrontMatter(frontMatter: string): Map { + const metadata = new Map(); + const lines = frontMatter.split(/\r?\n/g); + for (let index = 0; index < lines.length; index += 1) { + // Only top-level keys: anything indented belongs to the value above it. + const match = lines[index]!.match(/^([a-zA-Z0-9_-]+):\s*(.*?)\s*$/); + if (!match) continue; + const key = normalizeSkillMetadataKey(match[1]!); + const rawValue = match[2] ?? ""; + if (!SKILL_BLOCK_SCALAR_MARKERS.has(rawValue)) { + metadata.set(key, parseSkillMetadataValue(rawValue)); + continue; + } + + const body: string[] = []; + while (index + 1 < lines.length) { + const next = lines[index + 1]!; + // A blank line stays inside the block; anything back at the left margin ends it. + if (next.trim().length > 0 && !/^\s/.test(next)) break; + body.push(next.trim()); + index += 1; + } + const folded = rawValue.startsWith(">"); + metadata.set( + key, + folded ? body.filter((entry) => entry.length > 0).join(" ") : body.join("\n").trim(), + ); + } + return metadata; +} + function parseSkillMarkdown(input: { readonly name: string; readonly path: string; readonly contents: string; }) { const frontMatter = input.contents.match(/^---\r?\n([\s\S]*?)\r?\n---/); - const metadata = new Map(); - if (frontMatter) { - for (const line of frontMatter[1]!.split(/\r?\n/g)) { - const match = line.match(/^([a-zA-Z0-9_-]+):\s*(.*?)\s*$/); - if (!match) continue; - metadata.set(normalizeSkillMetadataKey(match[1]!), parseSkillMetadataValue(match[2] ?? "")); - } - } + const metadata = frontMatter ? parseSkillFrontMatter(frontMatter[1]!) : new Map(); const enabled = parseSkillMetadataBoolean(metadata.get("defaultenabled")) ?? true; return { name: optionalText(metadata.get("name")) ?? input.name, @@ -2500,6 +2738,109 @@ export function derivePluginBackedSkillBundle(skillPath: string): PluginBackedSk return null; } +/** + * Claude auto-loads each folder in the user skills dir as a plugin named `@skills-dir`, + * which is what `claude plugin enable|disable` takes and what lands in settings.json. + */ +export const CLAUDE_SKILLS_DIR_PLUGIN_SUFFIX = "@skills-dir"; + +/** + * A skill is only managed when its folder sits directly inside a root, `//SKILL.md`. + * Namespaced skills nested deeper belong to a layout Threadlines did not create, so they are left + * alone. Returns the skill's own directory when it matches. + */ +export function skillDirectoryUnderRoots( + path: Path.Path, + roots: ReadonlyArray, + skillPath: string, +): string | null { + const directory = path.dirname(path.resolve(skillPath)); + const parent = normalizedPathKey(path.dirname(directory)); + return roots.some((root) => normalizedPathKey(path.resolve(root)) === parent) ? directory : null; +} + +/** `enabledPlugins` in a Claude settings.json, ignoring anything that is not a boolean. */ +export function parseClaudeEnabledPlugins(contents: string): Map { + const enabled = new Map(); + if (contents.trim().length === 0) return enabled; + let parsed: unknown; + try { + parsed = JSON.parse(contents); + } catch { + return enabled; + } + if (typeof parsed !== "object" || parsed === null) return enabled; + const map = (parsed as { readonly enabledPlugins?: unknown }).enabledPlugins; + if (typeof map !== "object" || map === null) return enabled; + for (const [key, value] of Object.entries(map as Record)) { + if (typeof value === "boolean") enabled.set(key, value); + } + return enabled; +} + +/** + * `claude plugin list --json` does not report skills-dir entries (checked on 2.1.239), so the + * enabled state for a user skill comes from the settings.json map the CLI writes. A missing entry + * means the skill is enabled. + */ +const readClaudeEnabledPlugins = Effect.fn("providerExtensions.readClaudeEnabledPlugins")( + function* ( + claudeHome: string, + ): Effect.fn.Return, never, FileSystem.FileSystem | Path.Path> { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const contents = yield* fileSystem + .readFileString(path.join(claudeHome, ".claude", "settings.json")) + .pipe(Effect.catch(() => Effect.succeed(""))); + return parseClaudeEnabledPlugins(contents); + }, +); + +export function annotateClaudeSkillCapabilities( + skills: ReadonlyArray, + input: { + readonly path: Path.Path; + readonly userSkillsRoot: string; + readonly writableRoots: ReadonlyArray; + readonly enabledPlugins: ReadonlyMap; + }, +): ProviderExtensionSkill[] { + return skills.map((skill) => { + // A bundled skill follows the plugin that ships it; it is neither toggled nor deleted here. + if (skill.bundleId !== undefined) return skill; + const skillsDirDirectory = skillDirectoryUnderRoots( + input.path, + [input.userSkillsRoot], + skill.path, + ); + const pluginId = skillsDirDirectory + ? `${input.path.basename(skillsDirDirectory)}${CLAUDE_SKILLS_DIR_PLUGIN_SUFFIX}` + : null; + const canDelete = + skillDirectoryUnderRoots(input.path, input.writableRoots, skill.path) !== null; + return { + ...skill, + ...(pluginId ? { enabled: input.enabledPlugins.get(pluginId) ?? true, canToggle: true } : {}), + ...(canDelete ? { canDelete: true } : {}), + }; + }); +} + +export function annotateCodexSkillCapabilities( + skills: ReadonlyArray, + input: { readonly path: Path.Path; readonly userSkillsRoots: ReadonlyArray }, +): ProviderExtensionSkill[] { + return skills.map((skill) => { + if (skill.bundleId !== undefined) return skill; + // Codex toggles every skill it reports through `skills/config/write`. Its own bundled skills + // live a level deeper (`skills/.system//`), so the direct-child rule excludes them from + // deletion on its own. + const canDelete = + skillDirectoryUnderRoots(input.path, input.userSkillsRoots, skill.path) !== null; + return { ...skill, canToggle: true, ...(canDelete ? { canDelete: true } : {}) }; + }); +} + function annotatePluginBackedSkills( skills: ReadonlyArray, plugins: ReadonlyArray, @@ -2805,25 +3146,54 @@ export function claudePluginSkillRoots( }); } +/** The user skills root Claude auto-loads as `@skills-dir` plugins. */ +export function claudeUserSkillsRoot(path: Path.Path, claudeHome: string): string { + return path.join(claudeHome, ".claude", "skills"); +} + +/** + * Every skills root Threadlines is allowed to write to or delete from: the user root plus the + * project roots discovered around the cwd. Plugin-bundled roots are deliberately excluded — those + * belong to the plugin that installed them. + */ +const claudeWritableSkillRoots = Effect.fn("providerExtensions.claudeWritableSkillRoots")( + function* ( + claudeHome: string, + cwd: string, + ): Effect.fn.Return { + const path = yield* Path.Path; + const nestedProjectRoots = yield* discoverNestedClaudeSkillRoots(cwd); + // A project under the home directory walks its ancestors up through ~/.claude/skills, which + // would re-file every personal skill as a project skill. The user root keeps its own identity. + const userRootKey = normalizedPathKey(path.resolve(claudeUserSkillsRoot(path, claudeHome))); + const ancestorRoots = claudeAncestorSkillRoots(path, cwd).filter( + (root) => normalizedPathKey(path.resolve(root.root)) !== userRootKey, + ); + return uniqueClaudeSkillRoots( + [ + ...nestedProjectRoots, + ...ancestorRoots, + { + root: claudeUserSkillsRoot(path, claudeHome), + scope: "user", + source: "Claude user", + priority: 0, + }, + ], + path, + ); + }, +); + const readClaudeSkills = Effect.fn("providerExtensions.readClaudeSkills")(function* ( claudeHome: string, cwd: string, plugins: ReadonlyArray = [], ) { const path = yield* Path.Path; - const nestedProjectRoots = yield* discoverNestedClaudeSkillRoots(cwd); + const writableRoots = yield* claudeWritableSkillRoots(claudeHome, cwd); const skillRoots = uniqueClaudeSkillRoots( - [ - ...nestedProjectRoots, - ...claudeAncestorSkillRoots(path, cwd), - { - root: path.join(claudeHome, ".claude", "skills"), - scope: "user", - source: "Claude user", - priority: 0, - }, - ...claudePluginSkillRoots(path, plugins), - ], + [...writableRoots, ...claudePluginSkillRoots(path, plugins)], path, ); const discovered = yield* Effect.forEach(skillRoots, readSkillsFromRoot, { @@ -3198,7 +3568,8 @@ const readClaudeInventory = Effect.fn("providerExtensions.readClaudeInventory")( marketplaces, marketplaceManifests, ); - const skillsResult = yield* readClaudeSkills(path.resolve(claudeHome), input.cwd, plugins).pipe( + const resolvedClaudeHome = path.resolve(claudeHome); + const skillsResult = yield* readClaudeSkills(resolvedClaudeHome, input.cwd, plugins).pipe( Effect.result, ); const messages = [ @@ -3206,8 +3577,17 @@ const readClaudeInventory = Effect.fn("providerExtensions.readClaudeInventory")( resultMessage(mcpResult), resultMessage(skillsResult), ].filter((message): message is string => Boolean(message)); + const [enabledPlugins, writableRoots] = yield* Effect.all([ + readClaudeEnabledPlugins(resolvedClaudeHome), + claudeWritableSkillRoots(resolvedClaudeHome, input.cwd), + ]); const skills = Result.isSuccess(skillsResult) - ? annotatePluginBackedSkills(skillsResult.success, plugins) + ? annotateClaudeSkillCapabilities(annotatePluginBackedSkills(skillsResult.success, plugins), { + path, + userSkillsRoot: claudeUserSkillsRoot(path, resolvedClaudeHome), + writableRoots: writableRoots.map((root) => root.root), + enabledPlugins, + }) : []; return { status: messages.length > 0 ? "partial" : "ready", @@ -3601,6 +3981,241 @@ export const reloadProviderExtensionMcpServers = Effect.fn( return { reloaded: true }; }); +/** + * Which skills roots a provider lets Threadlines write to and delete from, and where a newly + * created skill goes. Resolved from settings alone — no provider process is started, because + * create and delete are plain filesystem work on both drivers. + */ +interface ProviderSkillRoots { + readonly driver: string; + /** Where `createProviderExtensionSkill` scaffolds. User scope only in v1. */ + readonly userSkillsRoot: string; + /** Every root a skill may be deleted from, user and project. */ + readonly writableRoots: ReadonlyArray; +} + +const resolveProviderSkillRoots = Effect.fn("providerExtensions.resolveProviderSkillRoots")( + function* (input: { + readonly cwd?: string | undefined; + readonly providerInstanceId: ProviderInstanceId; + readonly settings: ServerSettings; + }): Effect.fn.Return< + ProviderSkillRoots, + ProviderExtensionsError, + FileSystem.FileSystem | Path.Path + > { + const path = yield* Path.Path; + const cwd = input.cwd ?? machineScopeCwd(); + const providerConfig = yield* resolveProviderActionConfig(input); + + if (providerConfig.driver === CLAUDE_DRIVER) { + const decoded = yield* Effect.try({ + try: () => decodeClaudeSettings(providerConfig.config ?? {}), + catch: (cause) => + new ProviderExtensionsError({ + message: `Could not decode Claude settings for ${input.providerInstanceId}.`, + cause, + }), + }); + if (!(providerConfig.enabled ?? decoded.enabled)) { + return yield* new ProviderExtensionsError({ message: "Provider is disabled." }); + } + const claudeHome = yield* resolveClaudeHomePath(decoded); + const roots = yield* claudeWritableSkillRoots(claudeHome, cwd); + return { + driver: providerConfig.driver, + userSkillsRoot: claudeUserSkillsRoot(path, claudeHome), + writableRoots: roots.map((root) => root.root), + }; + } + + if (providerConfig.driver === CODEX_DRIVER) { + const decoded = yield* Effect.try({ + try: () => decodeCodexSettings(providerConfig.config ?? {}), + catch: (cause) => + new ProviderExtensionsError({ + message: `Could not decode Codex settings for ${input.providerInstanceId}.`, + cause, + }), + }); + if (!(providerConfig.enabled ?? decoded.enabled)) { + return yield* new ProviderExtensionsError({ message: "Provider is disabled." }); + } + const layout = yield* resolveCodexHomeLayout(decoded); + const roots = codexUserSkillsRoots(path, layout); + return { + driver: providerConfig.driver, + userSkillsRoot: roots[0]!, + writableRoots: roots, + }; + } + + return yield* new ProviderExtensionsError({ + message: "Managing skills is only available for Codex and Claude providers.", + }); + }, +); + +function skillMarkdownTemplate(input: { + readonly name: string; + readonly description: string; +}): string { + // JSON quoting doubles as a valid YAML double-quoted scalar, so a description with a colon or a + // quote in it cannot break the front matter. + return `--- +name: ${input.name} +description: ${JSON.stringify(input.description)} +--- + +# ${input.name} + +${input.description} + +Replace this with the steps the agent should follow. +`; +} + +export const createProviderExtensionSkill = Effect.fn( + "providerExtensions.createProviderExtensionSkill", +)(function* (input: { + readonly request: ProviderExtensionSkillCreateInput; + readonly settings: ServerSettings; +}): Effect.fn.Return< + ProviderExtensionSkillCreateResult, + ProviderExtensionsError, + FileSystem.FileSystem | Path.Path +> { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const name = input.request.name; + // The schema already enforces this, but the check is cheap and keeps a malformed name from ever + // reaching a path join. + if (!/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(name) || name.length > 64) { + return yield* new ProviderExtensionsError({ + message: + "Skill names use lower-case letters, digits, and single hyphens, up to 64 characters.", + }); + } + + const roots = yield* resolveProviderSkillRoots({ + cwd: input.request.cwd, + providerInstanceId: input.request.providerInstanceId, + settings: input.settings, + }); + const directory = path.join(roots.userSkillsRoot, name); + const exists = yield* fileSystem + .exists(directory) + .pipe(Effect.catch(() => Effect.succeed(false))); + if (exists) { + return yield* new ProviderExtensionsError({ + message: `A skill named ${name} already exists at ${directory}.`, + }); + } + + const skillPath = path.join(directory, "SKILL.md"); + const description = + optionalText(input.request.description) ?? `Describe when ${name} should be used.`; + yield* fileSystem.makeDirectory(directory, { recursive: true }).pipe( + Effect.andThen( + fileSystem.writeFileString(skillPath, skillMarkdownTemplate({ name, description })), + ), + Effect.mapError( + (cause) => + new ProviderExtensionsError({ + message: `Could not create the skill at ${skillPath}.`, + cause, + }), + ), + ); + return { path: skillPath }; +}); + +export const deleteProviderExtensionSkill = Effect.fn( + "providerExtensions.deleteProviderExtensionSkill", +)(function* (input: { + readonly request: ProviderExtensionSkillDeleteInput; + readonly settings: ServerSettings; +}): Effect.fn.Return< + ProviderExtensionSkillDeleteResult, + ProviderExtensionsError, + FileSystem.FileSystem | Path.Path +> { + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const skillPath = input.request.path; + if (!isCurrentProviderExtensionSkillPath(skillPath)) { + return yield* new ProviderExtensionsError({ + message: "Skill is not in the current provider extensions inventory.", + }); + } + if (derivePluginBackedSkillBundle(skillPath)) { + return yield* new ProviderExtensionsError({ + message: "This skill is installed by a plugin. Uninstall the plugin to remove it.", + }); + } + + const roots = yield* resolveProviderSkillRoots({ + cwd: input.request.cwd, + providerInstanceId: input.request.providerInstanceId, + settings: input.settings, + }); + + const linkTarget = yield* fileSystem + .readLink(skillPath) + .pipe(Effect.catch(() => Effect.succeed(null))); + if (linkTarget !== null) { + return yield* new ProviderExtensionsError({ + message: "Skill could not be deleted because the inventory entry is not a regular file.", + }); + } + const stat = yield* fileSystem + .stat(skillPath) + .pipe( + Effect.mapError(() => new ProviderExtensionsError({ message: "Skill could not be read." })), + ); + if (stat.type !== "File") { + return yield* new ProviderExtensionsError({ + message: "Skill could not be deleted because the inventory entry is not a regular file.", + }); + } + + // Compare real paths so a symlinked skills root (the Codex shadow home) resolves to the same + // place as the shared one, and so no `..` segment can walk out of a root. + const realDirectory = yield* fileSystem + .realPath(path.dirname(path.resolve(skillPath))) + .pipe( + Effect.mapError( + () => new ProviderExtensionsError({ message: "Skill directory could not be resolved." }), + ), + ); + const realRoots = yield* Effect.forEach(roots.writableRoots, (root) => + fileSystem.realPath(root).pipe(Effect.catch(() => Effect.succeed(null))), + ); + const parent = normalizedPathKey(path.dirname(realDirectory)); + const insideRoot = realRoots.some((root) => root !== null && normalizedPathKey(root) === parent); + if (!insideRoot) { + return yield* new ProviderExtensionsError({ + message: "Skill is not in a skills folder Threadlines manages.", + }); + } + + yield* fileSystem.remove(realDirectory, { recursive: true }).pipe( + Effect.mapError( + (cause) => + new ProviderExtensionsError({ + message: `Could not delete the skill folder ${realDirectory}.`, + cause, + }), + ), + ); + return { deleted: true }; +}); + +/** + * Codex has a first-class per-skill toggle. Claude has no skill toggle, but it auto-loads each + * folder in the user skills dir as a `@skills-dir` plugin, so the normal plugin + * enable/disable path reaches it. + */ export const setProviderExtensionSkillEnabled = Effect.fn( "providerExtensions.setProviderExtensionSkillEnabled", )(function* (input: { @@ -3611,6 +4226,36 @@ export const setProviderExtensionSkillEnabled = Effect.fn( ProviderExtensionsError, FileSystem.FileSystem | Path.Path | ChildProcessSpawner.ChildProcessSpawner > { + const providerConfig = yield* resolveProviderActionConfig({ + providerInstanceId: input.request.providerInstanceId, + settings: input.settings, + }); + + if (providerConfig.driver === CLAUDE_DRIVER) { + const path = yield* Path.Path; + const context = yield* resolveClaudeActionContext({ + cwd: input.request.cwd, + providerInstanceId: input.request.providerInstanceId, + settings: input.settings, + }); + const skillPath = optionalText(input.request.path); + const claudeHome = yield* resolveClaudeHomePath(context.config); + const directory = skillPath + ? skillDirectoryUnderRoots(path, [claudeUserSkillsRoot(path, claudeHome)], skillPath) + : null; + if (!directory) { + return yield* new ProviderExtensionsError({ + message: "Claude can only enable or disable skills in your personal skills folder.", + }); + } + yield* runClaudePluginAction(context, [ + "plugin", + input.request.enabled ? "enable" : "disable", + `${path.basename(directory)}${CLAUDE_SKILLS_DIR_PLUGIN_SUFFIX}`, + ]); + return { effectiveEnabled: input.request.enabled }; + } + const params = codexSkillConfigWriteParams(input.request); if (!params) { return yield* new ProviderExtensionsError({ @@ -3623,7 +4268,24 @@ export const setProviderExtensionSkillEnabled = Effect.fn( settings: input.settings, }); const response = yield* runCodexAppServerAction(context, (client) => - mapCodexRequestError(client.request("skills/config/write", params)), + Effect.gen(function* () { + // A failed listing must not block the toggle: writing the requested path alone is a partial + // result, which still beats refusing to change anything. + const shadowedPaths = yield* client.request("skills/list", { cwds: [context.cwd] }).pipe( + Effect.map((listed) => codexShadowedSkillWritePaths(listed, input.request)), + Effect.catch(() => Effect.succeed>([])), + ); + const written = yield* mapCodexRequestError(client.request("skills/config/write", params)); + yield* Effect.forEach( + shadowedPaths, + (path) => + mapCodexRequestError( + client.request("skills/config/write", { enabled: input.request.enabled, path }), + ), + { discard: true }, + ); + return written; + }), ); return { effectiveEnabled: response.effectiveEnabled }; }); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 393131b60..0d5a95153 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -88,6 +88,8 @@ import * as ProviderMaintenanceRunner from "./provider/providerMaintenanceRunner import { addProviderExtensionMarketplace, callProviderExtensionMcpTool, + createProviderExtensionSkill, + deleteProviderExtensionSkill, getProviderExtensionOperationStatus, installProviderExtensionPlugin, readProviderInstructionFiles, @@ -1368,6 +1370,24 @@ const makeWsRpcLayer = (currentSessionId: AuthSessionId) => }), { "rpc.aggregate": "server" }, ), + [WS_METHODS.serverCreateProviderExtensionSkill]: (input) => + observeRpcEffect( + WS_METHODS.serverCreateProviderExtensionSkill, + Effect.gen(function* () { + const settings = yield* loadProviderExtensionSettings; + return yield* createProviderExtensionSkill({ request: input, settings }); + }), + { "rpc.aggregate": "server" }, + ), + [WS_METHODS.serverDeleteProviderExtensionSkill]: (input) => + observeRpcEffect( + WS_METHODS.serverDeleteProviderExtensionSkill, + Effect.gen(function* () { + const settings = yield* loadProviderExtensionSettings; + return yield* deleteProviderExtensionSkill({ request: input, settings }); + }), + { "rpc.aggregate": "server" }, + ), [WS_METHODS.serverReadProviderExtensionPlugin]: (input) => observeRpcEffect( WS_METHODS.serverReadProviderExtensionPlugin, diff --git a/apps/web/src/components/CommandPalette.tsx b/apps/web/src/components/CommandPalette.tsx index 8b289790c..f279dbdc4 100644 --- a/apps/web/src/components/CommandPalette.tsx +++ b/apps/web/src/components/CommandPalette.tsx @@ -22,6 +22,7 @@ import { ArrowLeftIcon, ArrowUpIcon, CornerLeftUpIcon, + FileTextIcon, FolderIcon, FolderPlusIcon, GaugeIcon, @@ -1641,6 +1642,17 @@ function OpenCommandPaletteDialog() { }, }); + actionItems.push({ + kind: "action", + value: "action:skills", + searchTerms: ["skills", "plugins", "extensions", "agents", "capabilities"], + title: "Open skills", + icon: , + run: async () => { + await navigate({ to: "/settings/plugins", search: { tab: "skills" } }); + }, + }); + const rootGroups = buildRootGroups({ actionItems, recentThreadItems }); const activeGroups = currentView ? currentView.groups.map((group) => ({ diff --git a/apps/web/src/components/settings/ExtensionsSettings.logic.test.ts b/apps/web/src/components/settings/ExtensionsSettings.logic.test.ts index 44885e13b..13f6b1b67 100644 --- a/apps/web/src/components/settings/ExtensionsSettings.logic.test.ts +++ b/apps/web/src/components/settings/ExtensionsSettings.logic.test.ts @@ -14,14 +14,21 @@ import { extensionMcpNeedsAuthStatus, extensionMcpOAuthActionIntent, extensionMcpOAuthActionLabel, + extensionBrowserStatusLine, extensionTextMatchesFilter, extensionProviderDriverSortRank, formatExtensionGroupLabel, isLikelyLocalPath, + isProviderCoverageMissing, makeExtensionInventoryCacheKey, formatSkillDisplayName, formatTokenCount, + groupExtensionSkills, + bucketSkillsByPlugin, + dedupeShadowedSkills, groupPluginComponents, + parseExtensionsSettingsTab, + resolveExtensionsSettingsTab, makeExtensionJsonSchemaFormDefaults, resolvePluginComponentTarget, selectCuratedPlugins, @@ -31,6 +38,172 @@ import { } from "./ExtensionsSettings.logic"; describe("ExtensionsSettings logic", () => { + it("takes the tab from the URL and falls back to the remembered one", () => { + expect(parseExtensionsSettingsTab("skills")).toBe("skills"); + expect(parseExtensionsSettingsTab("nonsense")).toBeUndefined(); + expect(parseExtensionsSettingsTab(undefined)).toBeUndefined(); + + // An explicit link wins over whatever the last visit left behind. + expect(resolveExtensionsSettingsTab("plugins", "skills")).toBe("plugins"); + expect(resolveExtensionsSettingsTab(undefined, "skills")).toBe("skills"); + expect(resolveExtensionsSettingsTab(undefined, undefined)).toBe("plugins"); + }); + + it("groups skills by origin and sorts each group by name", () => { + const skills = [ + { name: "zebra", scope: "user" }, + { name: "shipped", scope: "system" }, + { name: "alpha", scope: "user" }, + { name: "repo-check", scope: "project" }, + { name: "bundled", scope: "user", bundleId: "helper@market" }, + ]; + + const groups = groupExtensionSkills(skills, (skill) => ({ + scope: skill.scope, + bundleId: skill.bundleId, + sortKey: skill.name, + })); + + expect(groups.map((group) => group.label)).toEqual([ + "Project skills", + "Personal skills", + "From plugins", + "Built in", + ]); + expect(groups[1]?.items.map((skill) => skill.name)).toEqual(["alpha", "zebra"]); + // A bundled skill belongs to its plugin no matter which root it was found in. + expect(groups[2]?.items.map((skill) => skill.name)).toEqual(["bundled"]); + }); + + it("hides a Codex built-in skill that the user has their own copy of", () => { + const skills = [ + { id: "user-imagegen", providerId: "codex", name: "imagegen", scope: "user" }, + { id: "system-imagegen", providerId: "codex", name: "imagegen", scope: "system" }, + { id: "system-openai-docs", providerId: "codex", name: "openai-docs", scope: "system" }, + // Same name, different provider: not a collision, both stay. + { id: "claude-imagegen", providerId: "claudeAgent", name: "imagegen", scope: "user" }, + ]; + + const visible = dedupeShadowedSkills(skills, (skill) => ({ + providerId: skill.providerId, + name: skill.name, + scope: skill.scope, + })); + + expect(visible.map((skill) => skill.id)).toEqual([ + "user-imagegen", + "system-openai-docs", + "claude-imagegen", + ]); + }); + + it("buckets bundled skills per plugin and opens only the narrowed ones", () => { + const skills = [ + { id: "a", bundle: "posthog@official", label: "PostHog", name: "alpha", matches: true }, + { id: "b", bundle: "posthog@official", label: "PostHog", name: "beta", matches: false }, + { id: "c", bundle: "figma@official", label: "Figma", name: "gamma", matches: true }, + { id: "d", bundle: "figma@official", label: "Figma", name: "delta", matches: true }, + { id: "e", bundle: "stripe@official", label: "Stripe", name: "eps", matches: false }, + ]; + + const buckets = bucketSkillsByPlugin(skills, (skill) => ({ + bundleId: skill.bundle, + label: skill.label, + sortKey: skill.name, + matches: skill.matches, + })); + + // Sorted by plugin name; the plugin with nothing matching drops out. + expect(buckets.map((bucket) => bucket.label)).toEqual(["Figma", "PostHog"]); + // Every Figma skill matched, so the query was about the plugin, not a skill inside it. + expect(buckets[0]?.autoExpand).toBe(false); + expect(buckets[0]?.matching.map((skill) => skill.name)).toEqual(["delta", "gamma"]); + // PostHog matched one of two, so the match is the point and the group opens. + expect(buckets[1]?.autoExpand).toBe(true); + expect(buckets[1]?.total).toBe(2); + expect(buckets[1]?.matching.map((skill) => skill.id)).toEqual(["a"]); + }); + + it("knows when the loaded inventory cannot answer for the selected provider", () => { + const base = { + providerInstanceId: "codex", + hasInventory: true, + hasError: false, + }; + + // The bug: a Claude-scoped fetch leaves an inventory with no Codex entry, so selecting the + // Codex chip filtered to nothing and every section claimed to be empty. + expect( + isProviderCoverageMissing({ ...base, inventoryProviderInstanceIds: ["claudeAgent"] }), + ).toBe(true); + + expect( + isProviderCoverageMissing({ + ...base, + inventoryProviderInstanceIds: ["codex", "claudeAgent"], + }), + ).toBe(false); + // No filter means the inventory covers whatever it returned. + expect( + isProviderCoverageMissing({ + ...base, + providerInstanceId: "", + inventoryProviderInstanceIds: ["claudeAgent"], + }), + ).toBe(false); + // Nothing loaded yet is the ordinary initial load, not a coverage gap. + expect( + isProviderCoverageMissing({ ...base, hasInventory: false, inventoryProviderInstanceIds: [] }), + ).toBe(false); + // A failed fetch keeps its own error presentation rather than pretending to load. + expect( + isProviderCoverageMissing({ ...base, hasError: true, inventoryProviderInstanceIds: [] }), + ).toBe(false); + }); + + it("says the browse catalog is still loading even when rows are already showing", () => { + const base = { + providerLabel: "Codex", + sectionTitle: "Apps", + visibleCount: 9, + totalCount: 9, + isCurated: false, + }; + + // The bug: 9 connected apps render immediately, so a plain count read as finished for the + // ~12s the directory took to arrive. + const loading = extensionBrowserStatusLine({ ...base, isLoading: true, hasError: false }); + expect(loading.state).toBe("loading"); + expect(loading.text).toBe("Codex - 9 shown, loading all apps"); + + const failed = extensionBrowserStatusLine({ ...base, isLoading: false, hasError: true }); + expect(failed.state).toBe("error"); + expect(failed.text).toBe("Codex - 9 shown, could not load all apps"); + + const ready = extensionBrowserStatusLine({ + ...base, + totalCount: 109, + isLoading: false, + hasError: false, + }); + expect(ready.state).toBe("ready"); + expect(ready.text).toBe("Codex - 9 visible from 109 total"); + + // Loading wins over an error left from a previous attempt. + expect(extensionBrowserStatusLine({ ...base, isLoading: true, hasError: true }).state).toBe( + "loading", + ); + }); + + it("drops skill groups that have no members", () => { + const groups = groupExtensionSkills([{ name: "solo", scope: "user" }], (skill) => ({ + scope: skill.scope, + sortKey: skill.name, + })); + + expect(groups.map((group) => group.key)).toEqual(["personal"]); + }); + it("matches extension records case-insensitively across provided fields", () => { expect(extensionTextMatchesFilter(["Browser", "Control the in-app browser"], "BROW")).toBe( true, diff --git a/apps/web/src/components/settings/ExtensionsSettings.logic.ts b/apps/web/src/components/settings/ExtensionsSettings.logic.ts index c979a07bc..b260b099f 100644 --- a/apps/web/src/components/settings/ExtensionsSettings.logic.ts +++ b/apps/web/src/components/settings/ExtensionsSettings.logic.ts @@ -22,6 +22,234 @@ export { type ExtensionMcpOAuthActionIntent, } from "../../mcpAuthStatus"; +// ── Page tabs ──────────────────────────────────────────────────────── +// +// The page holds two unrelated jobs — installing plugins and managing skills — so +// it splits into tabs. The tab lives in the URL so a link can point at either one, +// and is remembered so returning to settings lands where you left. + +export const EXTENSIONS_SETTINGS_TABS = ["plugins", "skills"] as const; +export type ExtensionsSettingsTab = (typeof EXTENSIONS_SETTINGS_TABS)[number]; + +/** Route search validation. Returns undefined for a missing or unknown tab. */ +export function parseExtensionsSettingsTab(value: unknown): ExtensionsSettingsTab | undefined { + return EXTENSIONS_SETTINGS_TABS.find((tab) => tab === value); +} + +/** The URL is explicit intent, so it beats the remembered tab. */ +export function resolveExtensionsSettingsTab( + searchTab: ExtensionsSettingsTab | undefined, + rememberedTab: ExtensionsSettingsTab | undefined, +): ExtensionsSettingsTab { + return searchTab ?? rememberedTab ?? "plugins"; +} + +// ── Skill grouping ─────────────────────────────────────────────────── + +export type ExtensionSkillGroupKey = "project" | "personal" | "plugin" | "builtin"; + +/** + * Where a skill came from, which is what a reader wants to know first: one they wrote for this + * project, one they wrote for themselves, one a plugin brought, or one the provider ships. + */ +export function extensionSkillGroupKey(skill: { + readonly scope?: string | undefined; + readonly bundleId?: string | undefined; +}): ExtensionSkillGroupKey { + if (skill.bundleId?.trim()) return "plugin"; + const scope = skill.scope?.trim().toLowerCase(); + if (scope === "project") return "project"; + if (scope === "system") return "builtin"; + return "personal"; +} + +const EXTENSION_SKILL_GROUP_ORDER = [ + "project", + "personal", + "plugin", + "builtin", +] as const satisfies ReadonlyArray; + +export const EXTENSION_SKILL_GROUP_LABELS: Record = { + project: "Project skills", + personal: "Personal skills", + plugin: "From plugins", + builtin: "Built in", +}; + +export interface ExtensionSkillGroup { + readonly key: ExtensionSkillGroupKey; + readonly label: string; + readonly items: ReadonlyArray; +} + +/** Groups in a fixed origin order, each sorted by the name shown on the row. Empty groups drop. */ +export function groupExtensionSkills( + items: ReadonlyArray, + read: (item: T) => { + readonly scope?: string | undefined; + readonly bundleId?: string | undefined; + readonly sortKey: string; + }, +): ReadonlyArray> { + return EXTENSION_SKILL_GROUP_ORDER.flatMap((key) => { + const matching = items.filter((item) => extensionSkillGroupKey(read(item)) === key); + if (matching.length === 0) return []; + return [ + { + key, + label: EXTENSION_SKILL_GROUP_LABELS[key], + items: matching.toSorted((left, right) => + read(left).sortKey.localeCompare(read(right).sortKey), + ), + }, + ]; + }); +} + +export interface ExtensionSkillIdentity { + readonly providerId: string; + readonly name: string; + readonly scope?: string | undefined; +} + +/** + * Codex ships built-in skills under its own `.system` root, and installing the same skill yourself + * leaves both reported under one name. The user's copy is the one that shadows the built-in and the + * only one with edit and delete affordances, so the built-in is dropped from the list. + */ +export function dedupeShadowedSkills( + items: ReadonlyArray, + read: (item: T) => ExtensionSkillIdentity, +): ReadonlyArray { + const key = (entry: ExtensionSkillIdentity) => + `${entry.providerId}\0${entry.name.trim().toLowerCase()}`; + const shadowedByUserCopy = new Set( + items.flatMap((item) => { + const entry = read(item); + return entry.scope?.trim().toLowerCase() === "user" ? [key(entry)] : []; + }), + ); + return items.filter((item) => { + const entry = read(item); + if (entry.scope?.trim().toLowerCase() !== "system") return true; + return !shadowedByUserCopy.has(key(entry)); + }); +} + +export interface ExtensionSkillPluginBucket { + readonly bundleId: string; + readonly label: string; + /** Every skill the plugin ships, matching or not. */ + readonly total: number; + readonly matching: ReadonlyArray; + /** + * The query narrowed this plugin to some of its skills, so the matches are the point and the + * group opens itself. A query that matches the plugin's own name matches all of them, which is + * not a reason to unfold a hundred rows. + */ + readonly autoExpand: boolean; +} + +/** + * One bucket per plugin, so a plugin shipping a hundred skills is one row until it is asked for. + * Buckets with nothing matching drop out entirely. + */ +export function bucketSkillsByPlugin( + items: ReadonlyArray, + read: (item: T) => { + readonly bundleId: string; + readonly label: string; + readonly sortKey: string; + readonly matches: boolean; + }, +): ReadonlyArray> { + const buckets = new Map(); + for (const item of items) { + const bundleId = read(item).bundleId; + const existing = buckets.get(bundleId); + if (existing) existing.push(item); + else buckets.set(bundleId, [item]); + } + + return [...buckets.entries()] + .flatMap(([bundleId, bucketItems]) => { + const matching = bucketItems + .filter((item) => read(item).matches) + .toSorted((left, right) => read(left).sortKey.localeCompare(read(right).sortKey)); + if (matching.length === 0) return []; + return [ + { + bundleId, + label: read(bucketItems[0]!).label, + total: bucketItems.length, + matching, + autoExpand: matching.length < bucketItems.length, + }, + ]; + }) + .toSorted((left, right) => left.label.localeCompare(right.label)); +} + +/** + * Whether the loaded inventory can say anything about the provider currently filtered to. A + * provider-scoped fetch only returns that provider, so selecting a different chip leaves an + * inventory that legitimately has no entry for it. Every section would then render its real empty + * label ("No plugins installed") while the refetch is still running, which reads as a definitive + * answer rather than a gap. A real error is not this case: the error presentation owns that. + */ +export function isProviderCoverageMissing(input: { + readonly providerInstanceId: string; + readonly inventoryProviderInstanceIds: ReadonlyArray; + readonly hasInventory: boolean; + readonly hasError: boolean; +}): boolean { + if (!input.providerInstanceId || !input.hasInventory || input.hasError) return false; + return !input.inventoryProviderInstanceIds.includes(input.providerInstanceId); +} + +export type ExtensionBrowserLoadState = "ready" | "loading" | "error"; + +export interface ExtensionBrowserStatusLine { + readonly state: ExtensionBrowserLoadState; + readonly text: string; +} + +/** + * What the Browse dialog says under its title. A deferred catalog is fetched when the dialog + * opens, and for apps that takes seconds while the already-connected rows are on screen. Without + * this the line reads "9 visible from 9 total" the whole time and the dialog looks finished. + */ +export function extensionBrowserStatusLine(input: { + readonly providerLabel: string; + readonly sectionTitle: string; + readonly visibleCount: number; + readonly totalCount: number; + readonly isCurated: boolean; + readonly isLoading: boolean; + readonly hasError: boolean; +}): ExtensionBrowserStatusLine { + const subject = input.sectionTitle.toLowerCase(); + if (input.isLoading) { + return { + state: "loading", + text: `${input.providerLabel} - ${input.visibleCount} shown, loading all ${subject}`, + }; + } + if (input.hasError) { + return { + state: "error", + text: `${input.providerLabel} - ${input.visibleCount} shown, could not load all ${subject}`, + }; + } + return { + state: "ready", + text: input.isCurated + ? `${input.providerLabel} - featured and most installed. Search to reach all ${input.totalCount}.` + : `${input.providerLabel} - ${input.visibleCount} visible from ${input.totalCount} total`, + }; +} + export function extensionProviderDriverSortRank(driverKind: string): number { if (driverKind === "codex") return 0; if (driverKind === "claudeAgent") return 1; diff --git a/apps/web/src/components/settings/ExtensionsSettings.tsx b/apps/web/src/components/settings/ExtensionsSettings.tsx index f8508e7cd..009b23e3c 100644 --- a/apps/web/src/components/settings/ExtensionsSettings.tsx +++ b/apps/web/src/components/settings/ExtensionsSettings.tsx @@ -21,8 +21,10 @@ import { RefreshCwIcon, SearchIcon, TerminalIcon, + Trash2Icon, WrenchIcon, } from "lucide-react"; +import { useNavigate, useSearch } from "@tanstack/react-router"; import { scopedThreadKey, scopeThreadRef } from "@threadlines/client-runtime"; import type { EnvironmentApi, @@ -80,12 +82,22 @@ import { extensionProviderDriverSortRank, formatSkillDisplayName, formatTokenCount, + groupExtensionSkills, + bucketSkillsByPlugin, + extensionBrowserStatusLine, + dedupeShadowedSkills, + EXTENSION_SKILL_GROUP_LABELS, + type ExtensionSkillPluginBucket, + parseExtensionsSettingsTab, + resolveExtensionsSettingsTab, + type ExtensionsSettingsTab, rankPluginsAcrossProviders, resolveExtensionScope, selectCuratedPlugins, shouldCuratePluginBrowse, groupPluginComponents, isLikelyLocalPath, + isProviderCoverageMissing, makeExtensionInventoryCacheKey, type ExtensionScopeGroup, type ExtensionScopeMachineInput, @@ -143,8 +155,6 @@ import { copyTextToClipboard } from "../../lib/clipboard"; import { cn } from "../../lib/utils"; const EXTENSION_SECTION_PREVIEW_LIMIT = 10; -/** Installed plugin lists are short now that the catalog lives behind Browse, so show them whole. */ -const EXTENSION_INSTALLED_PLUGIN_PREVIEW_LIMIT = 25; const EXTENSION_BROWSER_PAGE_SIZE = 80; const EXTENSION_INVENTORY_CACHE_MAX_ENTRIES = 5; const EXTENSION_INVENTORY_CACHE_TTL_MS = 10 * 60 * 1_000; @@ -222,6 +232,8 @@ interface ExtensionSectionConfig { readonly statusMessage?: string | undefined; readonly loadLabel?: string | undefined; readonly isLoading?: boolean | undefined; + /** The deferred load this section browses failed. Browse says so instead of failing silently. */ + readonly loadFailed?: boolean | undefined; readonly onLoad?: (() => void) | undefined; } @@ -243,6 +255,11 @@ interface ExtensionsSettingsPanelMemoryState { | undefined; manualThreadOverride?: { readonly scopeKey: string; readonly value: string } | undefined; showAdvancedContext?: boolean | undefined; + /** Overridden by a `tab` search param, so a shared link still opens where it points. */ + tab?: ExtensionsSettingsTab | undefined; + /** Plugin bundles left open on the Skills tab. */ + expandedSkillPlugins?: ReadonlyArray | undefined; + showMarketplaces?: boolean | undefined; } type ExtensionProvidersApi = NonNullable; @@ -358,35 +375,6 @@ function extensionItemActionKey(item: ExtensionItem): string { return `${item.provider.instanceId}:${item.kind}:${item.id}`; } -function statusVariant(status: ProviderExtensionProviderInventory["status"]) { - switch (status) { - case "ready": - return "success"; - case "partial": - return "warning"; - case "error": - return "error"; - case "disabled": - case "unsupported": - return "outline"; - } -} - -function providerStatusLabel(status: ProviderExtensionProviderInventory["status"]): string { - switch (status) { - case "ready": - return "Ready"; - case "partial": - return "Loaded with issues"; - case "error": - return "Error"; - case "disabled": - return "Disabled"; - case "unsupported": - return "Unsupported"; - } -} - function providerTitle(provider: ProviderExtensionProviderInventory): string { return provider.displayName ?? (provider.driver === "claudeAgent" ? "Claude" : provider.driver); } @@ -397,10 +385,12 @@ function inventoryHasLoadedMcpServers(inventory: ProviderExtensionsInventoryResu ); } +/** + * Whether the full app directory has been fetched. The connected apps come from a local snapshot + * on every load, so their presence says nothing about the catalog. + */ function inventoryHasLoadedApps(inventory: ProviderExtensionsInventoryResult): boolean { - return inventory.providers.some( - (provider) => provider.appsStatus === "ready" || provider.apps.length > 0, - ); + return inventory.providers.some((provider) => provider.appsCatalogStatus === "ready"); } function extensionKindLabel(kind: ExtensionItemKind): string { @@ -438,54 +428,50 @@ function formatBoolean(value: boolean | undefined): string | undefined { return value ? "Yes" : "No"; } -function SectionTabButton({ +/** + * The page's primary structure. Underline tabs rather than chips: chips sit next to the filter + * chips and read as one more filter, which is exactly what these are not. + */ +function PageTabButton({ label, - value, - totalValue, - isTruncated, - isDeferred, + count, active, - icon, panelId, onClick, }: { label: string; - value: number; - totalValue: number; - isTruncated?: boolean | undefined; - /** The section has not been fetched yet, so 0 would be a lie; show a placeholder instead. */ - isDeferred?: boolean | undefined; + count: number; active: boolean; - icon: ReactNode; panelId: string; onClick: () => void; }) { - const total = formatSectionTotal(totalValue, isTruncated); - const countLabel = isDeferred ? "–" : value === totalValue ? total : `${value}/${total}`; - return ( - + {label} + {count} + ); } +/** The provider's own mark, for rows and headers that belong to exactly one provider. */ +function ProviderNameGlyph({ driver }: { driver: string }) { + const Glyph = providerIconForDriverLabel(driver); + return Glyph ? : null; +} + function EmptyList({ label }: { label: string }) { return

{label}

; } @@ -638,6 +624,12 @@ function skillBundleKey(skill: ProviderExtensionSkill): string { }); } +/** `//SKILL.md` -> `/`: the folder a delete removes. */ +function skillFolderPath(skillPath: string): string { + const separatorIndex = skillPath.replaceAll("\\", "/").lastIndexOf("/"); + return separatorIndex > 0 ? skillPath.slice(0, separatorIndex) : skillPath; +} + function findRefreshedExtensionItem( current: ExtensionItem, inventory: ProviderExtensionsInventoryResult, @@ -900,7 +892,8 @@ function ExtensionItemBadges({ ) : null} {extensionItemInstalled(item) ? ( - Installed + {/* Nothing is installed for an app; it is a connection to a ChatGPT account. */} + {item.kind === "app" ? "Connected" : "Installed"} ) : null} {extensionItemIsOfficial(item) ? ( @@ -1542,6 +1535,7 @@ function ExtensionDetailDialog({ onSelectItem, environmentId, cwd, + machineLabel, providerThreadId, onInventoryMutated, lastAction, @@ -1552,6 +1546,8 @@ function ExtensionDetailDialog({ onSelectItem: (item: ExtensionItem) => void; environmentId: EnvironmentId | null; cwd: string; + /** Named in the delete confirmation so it is clear which machine loses the folder. */ + machineLabel: string; providerThreadId: string; onInventoryMutated: () => Promise; lastAction?: ExtensionActionHistoryEntry | undefined; @@ -1806,6 +1802,25 @@ function ExtensionDetailDialog({ }); }, [cwd, item, onInventoryMutated, providersApi, runDialogAction]); + const deleteSkill = useCallback(() => { + if (!item || item.kind !== "skill") return; + const folder = skillFolderPath(item.skill.path); + void runDialogAction("Delete skill", async () => { + const confirmed = await ensureLocalApi().dialogs.confirm( + `Delete ${item.title}? Deletes the folder ${folder} on ${machineLabel}.`, + ); + if (!confirmed) return "Delete cancelled."; + await providersApi().deleteExtensionSkill({ + ...actionBaseInput(item, cwd), + path: item.skill.path, + }); + await onInventoryMutated(); + // The item this dialog is bound to no longer exists, so there is nothing left to show. + onClose(); + return "Skill deleted."; + }); + }, [cwd, item, machineLabel, onClose, onInventoryMutated, providersApi, runDialogAction]); + const installPlugin = useCallback(() => { if (!item || item.kind !== "plugin") return; void runDialogAction("Install plugin", async () => { @@ -2161,7 +2176,7 @@ function ExtensionDetailDialog({ - + ) : null} @@ -2414,7 +2429,7 @@ function ExtensionDetailDialog({ ) : null} ) : null} - {codexActionsAvailable && item.kind === "skill" ? ( + {item.kind === "skill" && item.skill.canToggle === true ? ( + ) : null} {codexActionsAvailable && item.kind === "plugin" ? ( <> {item.plugin.installed === true ? ( @@ -2702,7 +2732,7 @@ function ExtensionPreviewSection({ {visibleItems.map((item) => ( + onToggle(item, Boolean(checked))} + /> + + + ); +} + +function InstalledPluginsList({ items, environmentId, + busyPluginId, onSelect, + onToggle, }: { - items: ReadonlyArray; + items: ReadonlyArray>; environmentId: EnvironmentId | null; + busyPluginId: string | null; onSelect: (item: ExtensionItem) => void; + onToggle: (item: Extract, nextEnabled: boolean) => void; }) { - if (items.length === 0) return null; + if (items.length === 0) return ; return ( -
+
{items.map((item) => ( - - onSelect(item)} - aria-label={item.title} - > - {/* Plugin artwork already ships its own tile and background, so wrapping it in - another square just shrinks it and fights whatever the icon draws. Only the - fallback needs a container of ours. */} - - - - } - /> - - } - /> - {item.title} - + ))}
); } -function InstalledStripSkeleton() { +function InstalledPluginsSkeleton() { return ( -