diff --git a/docs/chatgpt-coding-workflow.md b/docs/chatgpt-coding-workflow.md index 4e001e46c..09d85aef8 100644 --- a/docs/chatgpt-coding-workflow.md +++ b/docs/chatgpt-coding-workflow.md @@ -136,12 +136,11 @@ before use. Legacy project paths such as `.pi/skills` can be added to `skills.paths` when needed. -When `open_workspace` returns matching skills, the model should read the -advertised `SKILL.md` before following that skill. - -Skill paths may be outside the workspace. DevSpace only permits reading: - -- files within advertised skill directories +When `open_workspace` returns matching skills, the model reads the advertised +skill path before following that skill. For early testing, +`DEVSPACE_EXPERIMENTAL_SKILL_URIS=1` replaces physical skill paths with +`skills://` URIs; bundled resources then use +`skills:///`. Set `skills.enabled` to `false` to hide skills from workspace output. Enable Subagents and choose providers through `devspace init` or the persisted provider diff --git a/docs/configuration.md b/docs/configuration.md index 408018c7f..9c2d6e9ae 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -114,6 +114,16 @@ DevSpace discovers standard Agent Skills from `~/.agents/skills`, project `skills.agentDir/skills` and each path in `skills.paths`. Relative custom paths are resolved from the active workspace. +By default, discovered skills keep their filesystem paths for compatibility +with existing MCP hosts. Set `DEVSPACE_EXPERIMENTAL_SKILL_URIS=1` to expose +logical `skills://` URIs instead. The bare URI loads the skill entry +file; bundled resources use `skills:///`. +As an experimental compatibility workaround, standalone skill URI arguments in +the shell tools are resolved to their local files immediately before execution. + +Skill URIs are experimental and may be removed in favor of the MCP Skills +extension as host support matures. + When Subagents are enabled for MCP workspaces, DevSpace keeps its bundled `subagents` skill synchronized at `~/.devspace/skills/subagents/SKILL.md`. That managed copy is the authoritative `subagents` skill for DevSpace and is diff --git a/docs/gotchas.md b/docs/gotchas.md index cb660c732..4ad28d5b3 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -260,9 +260,10 @@ Copy or adapt them into one of the active profile directories before use. Legacy project paths such as `.pi/skills` can be added to `skills.paths` when needed. -If a skill appears in `open_workspace`, the model should read that skill's -`SKILL.md` before following it. DevSpace permits reads within advertised skill -directories without tracking whether `SKILL.md` was read first. +If a skill appears in `open_workspace`, the model reads its advertised path +before following it. With `DEVSPACE_EXPERIMENTAL_SKILL_URIS=1`, DevSpace +instead advertises `skills://`; bundled resources use +`skills:///`. ## Review Card Does Not Appear diff --git a/src/config.test.ts b/src/config.test.ts index ddb2fc1c3..e276d0a11 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -21,6 +21,7 @@ try { assert.equal(defaults.toolMode, "codex"); assert.equal(defaults.uiEnabled, true); assert.equal(defaults.skillsEnabled, true); + assert.equal(defaults.experimentalSkillUris, false); assert.equal(defaults.artifactsEnabled, false); assert.deepEqual(defaults.subagents, { enabled: false, @@ -121,6 +122,10 @@ try { }); assert.equal(loadConfig(env).oauth.ownerToken, env.DEVSPACE_OAUTH_OWNER_TOKEN); + assert.equal( + loadConfig({ ...env, DEVSPACE_EXPERIMENTAL_SKILL_URIS: "1" }).experimentalSkillUris, + true, + ); } finally { rmSync(configDir, { recursive: true, force: true }); } diff --git a/src/config.ts b/src/config.ts index 34fcdfc25..7fc753787 100644 --- a/src/config.ts +++ b/src/config.ts @@ -23,6 +23,7 @@ export interface ServerConfig { artifactsEnabled: boolean; artifactMaxFileBytes: number; skillsEnabled: boolean; + experimentalSkillUris: boolean; skillPaths: string[]; devspaceSkillsDir: string; devspaceAgentsDir: string; @@ -72,6 +73,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig { artifactsEnabled: stored.artifacts.enabled, artifactMaxFileBytes: stored.artifacts.maxFileBytes, skillsEnabled: stored.skills.enabled, + experimentalSkillUris: env.DEVSPACE_EXPERIMENTAL_SKILL_URIS === "1", skillPaths: stored.skills.paths, devspaceSkillsDir: devspaceSkillsDir(env), devspaceAgentsDir: devspaceAgentsDir(env), diff --git a/src/server.test.ts b/src/server.test.ts index 59bcc5d37..552c36512 100644 --- a/src/server.test.ts +++ b/src/server.test.ts @@ -435,10 +435,65 @@ test("open_workspace advertises subagent instructions on demand by default", asy const opened = structuredContent(await callOpen(context.client, context.project, "chat-1")); const skills = opened.skills as Array>; - assert.equal(skills.some((skill) => skill.name === "subagents"), true); + const subagents = skills.find((skill) => skill.name === "subagents"); + assert.ok(subagents); + assert.match(String(subagents.path), /\/skills\/subagents\/SKILL\.md$/); assert.doesNotMatch(String(opened.instruction), /# DevSpace subagents/); }); +test("open_workspace advertises experimental skill URIs when enabled", async (t) => { + const context = await fixture(t, { + localAgentProviders: [{ name: "codex", available: true }], + experimentalSkillUris: true, + }); + + const opened = structuredContent(await callOpen(context.client, context.project, "chat-1")); + const skills = opened.skills as Array>; + const subagents = skills.find((skill) => skill.name === "subagents"); + assert.ok(subagents); + assert.equal(subagents.path, "skills://subagents"); + assert.doesNotMatch(String(opened.instruction), /# DevSpace subagents/); +}); + +test("experimental skill URIs work as shell command arguments", async (t) => { + for (const toolMode of ["codex", "claude"] as const) { + await t.test(toolMode, async (t) => { + const context = await fixture(t, { + toolMode, + localAgentProviders: [{ name: "codex", available: true }], + experimentalSkillUris: true, + }); + const workspaceId = structuredContent( + await callOpen(context.client, context.project, `skill-shell-${toolMode}`), + ).workspace_id; + assert.equal(typeof workspaceId, "string"); + + const skillArgument = toolMode === "codex" + ? "skills://subagents" + : '"skills://subagents"'; + const command = + `node -p "require('node:fs').readFileSync(process.argv[1],'utf8')" ${skillArgument}`; + const result = structuredContent(await context.client.callTool({ + name: toolMode === "codex" ? "exec_command" : "bash", + arguments: toolMode === "codex" + ? { workspace_id: workspaceId, cmd: command } + : { workspace_id: workspaceId, command }, + })); + + assert.match(String(result.result), /# DevSpace subagents/); + + const literalCommand = `node -p "process.argv[1]" "prefixskills://subagents"`; + const literal = structuredContent(await context.client.callTool({ + name: toolMode === "codex" ? "exec_command" : "bash", + arguments: toolMode === "codex" + ? { workspace_id: workspaceId, cmd: literalCommand } + : { workspace_id: workspaceId, command: literalCommand }, + })); + assert.match(String(literal.result), /prefixskills:\/\/subagents/); + }); + } +}); + test("open_workspace preloads subagent instructions when configured", async (t) => { const context = await fixture(t, { localAgentProviders: [{ name: "codex", available: true }], @@ -722,6 +777,7 @@ async function fixture( subagents?: SubagentsConfig; toolMode?: ToolMode; uiEnabled?: boolean; + experimentalSkillUris?: boolean; } = {}, ): Promise { const root = await mkdtemp(join(tmpdir(), "devspace-server-test-")); @@ -754,7 +810,7 @@ async function fixture( const initialProviderAvailability = typeof options.localAgentProviders === "function" ? options.localAgentProviders() : options.localAgentProviders ?? []; - const loadedConfig = loadConfig(writeTestDevspaceConfig(join(root, ".config"), { + const configEnv = writeTestDevspaceConfig(join(root, ".config"), { server: { port: 1 }, workspaces: { allowedRoots: [root], worktreeRoot: join(root, ".worktrees") }, skills: { agentDir }, @@ -763,7 +819,13 @@ async function fixture( instructions: "on-demand", providers: [], }, - })); + }); + const loadedConfig = loadConfig({ + ...configEnv, + ...(options.experimentalSkillUris + ? { DEVSPACE_EXPERIMENTAL_SKILL_URIS: "1" } + : {}), + }); const modeConfig: ServerConfig = { ...loadedConfig, toolMode: options.toolMode ?? loadedConfig.toolMode, diff --git a/src/server.ts b/src/server.ts index 0a2792f3f..4d5831de9 100644 --- a/src/server.ts +++ b/src/server.ts @@ -43,7 +43,7 @@ import { ProcessSessionManager } from "./process-sessions.js"; import { createReviewCheckpointManager } from "./review-checkpoints.js"; import { conversationScopeIdFromRequestMeta } from "./request-meta.js"; import { shutdownHttpServer } from "./server-shutdown.js"; -import { formatPathForPrompt } from "./skills.js"; +import { formatPathForPrompt, formatSkillUri } from "./skills.js"; import { DEVSPACE_VERSION } from "./version.js"; import { createWorkspaceStore } from "./workspace-store.js"; import { formatAgentsPath, WorkspaceRegistry } from "./workspaces.js"; @@ -131,7 +131,7 @@ function serverInstructions( const showChangesInstruction = " If files are modified, call show_changes once after the final related change and before the final response."; const skills = config.skillsEnabled - ? `When ${toolNames.openWorkspace} returns available skills and a task matches one, use ${toolNames.read} with the returned skill path before proceeding. ` + ? `When ${toolNames.openWorkspace} returns available skills and a task matches one, use ${toolNames.read} with the returned ${skillReferenceLabel(config)} before proceeding. ` : ""; const agents = `Follow instructions returned by ${toolNames.openWorkspace}. Before working under a path listed in available_agents_files, use ${toolNames.read} to inspect that instruction file and follow it. `; const common = `Call ${toolNames.openWorkspace} when starting work in a project folder or isolated worktree without a usable workspace_id, then reuse the returned workspace_id for subsequent operations in that workspace.`; @@ -139,6 +139,10 @@ function serverInstructions( return `${common} ${toolSurface.instructions({ agents, skills })}${artifactInstruction}${showChangesInstruction}`; } +function skillReferenceLabel(config: ServerConfig): string { + return config.experimentalSkillUris ? "skills:// URI" : "skill path"; +} + function formatVisibleAgent(agent: { name: string; provider: string; @@ -467,7 +471,9 @@ function registerMcpSurface( .map((skill) => ({ name: skill.name, description: skill.description, - path: formatPathForPrompt(skill.filePath), + path: config.experimentalSkillUris + ? formatSkillUri(skill) + : formatPathForPrompt(skill.filePath), })); const agentCatalog = buildLocalAgentCatalog( config.subagents, @@ -496,7 +502,7 @@ function registerMcpSurface( const loadedAgentsFiles = includeBootstrapContext ? cardAgentsFiles : []; const availableAgentsFileOutputs = includeBootstrapContext ? cardAvailableAgentsFiles : []; const cardInstruction = config.skillsEnabled - ? "Use this workspace_id for subsequent work in this project. Keep reusing it while working in this project. Follow loaded agents_files instructions. Before working under a path listed in available_agents_files, read that instruction file. When a task matches an available skill in skills, read its path before proceeding." + ? `Use this workspace_id for subsequent work in this project. Keep reusing it while working in this project. Follow loaded agents_files instructions. Before working under a path listed in available_agents_files, read that instruction file. When a task matches an available skill in skills, read its ${skillReferenceLabel(config)} before proceeding.` : "Use this workspace_id for subsequent work in this project. Keep reusing it while working in this project. Follow loaded agents_files instructions. Before working under a path listed in available_agents_files, read that instruction file."; const workspaceInstruction = workspaceReused ? [ @@ -622,7 +628,7 @@ function registerMcpSurface( "Read all or part of a file in a workspace.", "Use this tool to inspect relevant AGENTS.md or CLAUDE.md files listed by open_workspace before working in nested directories.", config.skillsEnabled - ? "If available skills were returned and a task matches one, read the returned skill path before proceeding." + ? `If available skills were returned and a task matches one, read the returned ${skillReferenceLabel(config)} before proceeding.` : "", ] .filter(Boolean) @@ -635,7 +641,7 @@ function registerMcpSurface( .string() .describe( config.skillsEnabled - ? "File path relative to the workspace root, or a skill path returned by open_workspace." + ? `File path relative to the workspace root, or a ${skillReferenceLabel(config)} returned by open_workspace.` : "File path to read, relative to the workspace root.", ), offset: z diff --git a/src/skills.test.ts b/src/skills.test.ts index 357c417db..017359876 100644 --- a/src/skills.test.ts +++ b/src/skills.test.ts @@ -6,6 +6,7 @@ import { loadConfig } from "./config.js"; import { effectiveSkillPaths, formatPathForPrompt, + formatSkillUri, loadWorkspaceSkills, resolveSkillReadPath, } from "./skills.js"; @@ -35,6 +36,7 @@ try { await mkdir(join(agentDir, "skills", "subagents"), { recursive: true }); await mkdir(join(explicitSkills, "duplicate"), { recursive: true }); await mkdir(join(explicitSkills, "disabled"), { recursive: true }); + await mkdir(join(explicitSkills, "invalid-name"), { recursive: true }); await mkdir(join(explicitSkills, "subagents"), { recursive: true }); await mkdir(join(devspaceSkills, "devspace-local-skill"), { recursive: true }); @@ -160,6 +162,17 @@ try { "# Hidden Skill", ].join("\n"), ); + await writeFile( + join(explicitSkills, "invalid-name", "SKILL.md"), + [ + "---", + "name: foo/bar", + "description: Invalid skill name.", + "---", + "", + "# Invalid Skill", + ].join("\n"), + ); const configDir = join(root, ".devspace"); const disabledConfig = loadConfig(writeTestDevspaceConfig(configDir, { @@ -169,14 +182,22 @@ try { })); assert.deepEqual(loadWorkspaceSkills(disabledConfig, projectRoot).skills, []); - const config = loadConfig(writeTestDevspaceConfig(configDir, { + const configEnv = writeTestDevspaceConfig(configDir, { server: { port: 1 }, workspaces: { allowedRoots: [projectRoot] }, skills: { agentDir, paths: [explicitSkills, "~/.claude/skills", "./.claude/skills"], }, - })); + }); + const defaultConfig = loadConfig(configEnv); + const defaultLoaded = loadWorkspaceSkills(defaultConfig, projectRoot); + assert.equal(defaultLoaded.skills.some((skill) => skill.name === "foo/bar"), true); + + const config = loadConfig({ + ...configEnv, + DEVSPACE_EXPERIMENTAL_SKILL_URIS: "1", + }); const loaded = loadWorkspaceSkills(config, projectRoot); assert.equal(loaded.skills.some((skill) => skill.name === "agent-global-skill"), true); assert.equal(loaded.skills.some((skill) => skill.name === "agent-project-skill"), true); @@ -187,7 +208,14 @@ try { assert.equal(loaded.skills.some((skill) => skill.name === "subagents"), false); assert.equal(loaded.skills.filter((skill) => skill.name === "duplicate-skill").length, 1); assert.equal(loaded.skills.some((skill) => skill.name === "hidden-skill"), true); + assert.equal(loaded.skills.some((skill) => skill.name === "foo/bar"), false); assert.equal(loaded.diagnostics.some((diagnostic) => diagnostic.type === "collision"), true); + assert.equal( + loaded.diagnostics.some( + (diagnostic) => diagnostic.message.includes("name contains invalid characters"), + ), + true, + ); assert.equal( loaded.diagnostics.some( (diagnostic) => diagnostic.collision?.name === "subagents", @@ -247,14 +275,60 @@ try { const projectSkill = loaded.skills.find((skill) => skill.name === "agent-project-skill"); assert.ok(projectSkill); - assert.match(formatPathForPrompt(projectSkill.filePath), /SKILL\.md$/); + const defaultProjectSkill = defaultLoaded.skills.find( + (skill) => skill.name === "agent-project-skill", + ); + assert.ok(defaultProjectSkill); + assert.match(formatPathForPrompt(defaultProjectSkill.filePath), /SKILL\.md$/); + assert.equal( + resolveSkillReadPath(defaultLoaded.skills, defaultProjectSkill.filePath, false)?.absolutePath, + defaultProjectSkill.filePath, + ); + assert.equal( + resolveSkillReadPath(defaultLoaded.skills, "skills://agent-project-skill", false), + undefined, + ); - const skillFileRead = resolveSkillReadPath(loaded.skills, projectSkill.filePath); + assert.equal(formatSkillUri(projectSkill), "skills://agent-project-skill"); + assert.throws( + () => formatSkillUri({ ...projectSkill, name: "foo/bar" }), + /Invalid skill name/, + ); + + const skillFileRead = resolveSkillReadPath( + loaded.skills, + "skills://agent-project-skill", + true, + ); assert.equal(skillFileRead?.absolutePath, projectSkill.filePath); const resourcePath = join(projectSkill.baseDir, "references.md"); await writeFile(resourcePath, "reference\n"); - assert.equal(resolveSkillReadPath(loaded.skills, resourcePath)?.absolutePath, resourcePath); + assert.equal( + resolveSkillReadPath( + loaded.skills, + "skills://agent-project-skill/references.md", + true, + )?.absolutePath, + resourcePath, + ); + assert.equal(resolveSkillReadPath(loaded.skills, projectSkill.filePath, true), undefined); + assert.throws( + () => resolveSkillReadPath(loaded.skills, "skills://missing", true), + /Unknown skill/, + ); + assert.throws( + () => resolveSkillReadPath(loaded.skills, "skills://foo%2Fbar", true), + /Invalid skill URI/, + ); + assert.throws( + () => resolveSkillReadPath( + loaded.skills, + "skills://agent-project-skill/../secret", + true, + ), + /outside skill directory/, + ); } finally { if (originalHome === undefined) delete process.env.HOME; else process.env.HOME = originalHome; diff --git a/src/skills.ts b/src/skills.ts index 8f67d8e07..acf8d2046 100644 --- a/src/skills.ts +++ b/src/skills.ts @@ -29,6 +29,9 @@ export interface SkillReadResolution { skill: Skill; } +const SKILL_URI_PREFIX = "skills://"; +const MAX_SKILL_NAME_LENGTH = 64; +const SKILL_NAME_PATTERN = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; const SUBAGENTS_SKILL_NAME = "subagents"; const SUBAGENTS_SKILL = join(SUBAGENTS_SKILL_NAME, "SKILL.md"); @@ -108,7 +111,13 @@ export function loadWorkspaceSkills(config: ServerConfig, cwd: string): LoadedSk }); const withoutSubagents = withoutSubagentsSkill(result); - if (!config.subagents.enabled) return withoutSubagents; + const available = config.experimentalSkillUris + ? { + skills: withoutSubagents.skills.filter((skill) => isRoutableSkillName(skill.name)), + diagnostics: withoutSubagents.diagnostics, + } + : withoutSubagents; + if (!config.subagents.enabled) return available; const managedDir = dirname(join(config.devspaceSkillsDir, SUBAGENTS_SKILL)); const managed = loadSkillsFromDir({ @@ -120,8 +129,8 @@ export function loadWorkspaceSkills(config: ServerConfig, cwd: string): LoadedSk } return { - skills: [...withoutSubagents.skills, managed], - diagnostics: withoutSubagents.diagnostics, + skills: [...available.skills, managed], + diagnostics: available.diagnostics, }; } @@ -138,24 +147,69 @@ function withoutSubagentsSkill(result: LoadSkillsResult): LoadedSkills { export function resolveSkillReadPath( skills: Skill[], inputPath: string, + experimentalSkillUris: boolean, ): SkillReadResolution | undefined { - const absolutePath = resolve(expandHomePath(inputPath)); + if (!experimentalSkillUris) { + const absolutePath = resolve(expandHomePath(inputPath)); + + for (const skill of skills) { + const skillFilePath = resolve(skill.filePath); + if (absolutePath === skillFilePath) { + return { absolutePath, skill }; + } + } + + for (const skill of skills) { + const baseDir = resolve(skill.baseDir); + if (!isPathInsideRoot(absolutePath, baseDir)) continue; - for (const skill of skills) { - const skillFilePath = resolve(skill.filePath); - if (absolutePath === skillFilePath) { return { absolutePath, skill }; } + + return undefined; + } + + if (!inputPath.startsWith(SKILL_URI_PREFIX)) return undefined; + + const skillReference = inputPath.slice(SKILL_URI_PREFIX.length); + const separatorIndex = skillReference.indexOf("/"); + const skillName = separatorIndex === -1 + ? skillReference + : skillReference.slice(0, separatorIndex); + const resourcePath = separatorIndex === -1 + ? undefined + : skillReference.slice(separatorIndex + 1); + + if (!skillName) { + throw new Error(`Invalid skill URI: ${inputPath}`); + } + if (!isRoutableSkillName(skillName)) { + throw new Error(`Invalid skill URI: ${inputPath}`); } - for (const skill of skills) { - const baseDir = resolve(skill.baseDir); - if (!isPathInsideRoot(absolutePath, baseDir)) continue; + const skill = skills.find((candidate) => candidate.name === skillName); + if (!skill) { + throw new Error(`Unknown skill: ${skillName}`); + } + + if (!resourcePath) { + return { absolutePath: resolve(skill.filePath), skill }; + } - return { absolutePath, skill }; + const baseDir = resolve(skill.baseDir); + const absolutePath = resolve(baseDir, resourcePath); + if (!isPathInsideRoot(absolutePath, baseDir)) { + throw new Error(`Skill resource is outside skill directory: ${inputPath}`); } - return undefined; + return { absolutePath, skill }; +} + +export function formatSkillUri(skill: Skill): string { + if (!isRoutableSkillName(skill.name)) { + throw new Error(`Invalid skill name for skills:// URI: ${skill.name}`); + } + return `${SKILL_URI_PREFIX}${skill.name}`; } export function formatPathForPrompt(path: string): string { @@ -169,3 +223,7 @@ export function formatPathForPrompt(path: string): string { return resolvedPath.split(sep).join("/"); } + +function isRoutableSkillName(name: string): boolean { + return name.length <= MAX_SKILL_NAME_LENGTH && SKILL_NAME_PATTERN.test(name); +} diff --git a/src/tool-surfaces/claude.ts b/src/tool-surfaces/claude.ts index ffb083ab5..241397cb6 100644 --- a/src/tool-surfaces/claude.ts +++ b/src/tool-surfaces/claude.ts @@ -16,6 +16,7 @@ import { import { contentText, countDiffStats, + expandSkillUrisInShellCommand, logFailedToolResponse, logToolCall, resultOutputSchema, @@ -182,7 +183,11 @@ function registerShellTool(context: ToolRegistrationContext): void { toolNames.shell, { title: "Bash", - description: CLAUDE_SHELL_DESCRIPTION, + description: + CLAUDE_SHELL_DESCRIPTION + + (config.experimentalSkillUris + ? " Standalone skills:// arguments are resolved before execution." + : ""), inputSchema: { workspace_id: z.string().describe(workspaceIdDescription), command: z @@ -213,7 +218,14 @@ function registerShellTool(context: ToolRegistrationContext): void { workspace, workingDirectory, ); - const response = await runShellTool(input, { + const command = await expandSkillUrisInShellCommand( + config, + workspaces, + workspace, + input.command, + "bash", + ); + const response = await runShellTool({ ...input, command }, { cwd, }); diff --git a/src/tool-surfaces/codex.ts b/src/tool-surfaces/codex.ts index 9770897cb..13f15d352 100644 --- a/src/tool-surfaces/codex.ts +++ b/src/tool-surfaces/codex.ts @@ -14,6 +14,7 @@ import { } from "./types.js"; import { contentText, + expandSkillUrisInShellCommand, resultOutputSchema, runLoggedToolOperation, textBlock, @@ -147,7 +148,11 @@ function registerCodexProcessTools(context: ToolRegistrationContext): void { { title: "Execute command", description: - "Run a shell command in a workspace with the user's local permissions. Returns the result when it exits during the yield window, otherwise returns a session_id for write_stdin.", + "Run a shell command in a workspace with the user's local permissions. " + + (config.experimentalSkillUris + ? "Standalone skills:// arguments are resolved before execution. " + : "") + + "Returns the result when it exits during the yield window, otherwise returns a session_id for write_stdin.", inputSchema: { workspace_id: z.string().describe(workspaceIdDescription), cmd: z.string().min(1).describe("Shell command to execute."), @@ -228,9 +233,16 @@ function registerCodexProcessTools(context: ToolRegistrationContext): void { workspace, workingDirectory, ); + const command = await expandSkillUrisInShellCommand( + config, + workspaces, + workspace, + cmd, + "native", + ); return processSessions.start({ workspaceId, - command: cmd, + command, cwd, workspaceRoot: workspace.root, tty, diff --git a/src/tool-surfaces/shared.ts b/src/tool-surfaces/shared.ts index 1366d5a97..c3d02a357 100644 --- a/src/tool-surfaces/shared.ts +++ b/src/tool-surfaces/shared.ts @@ -1,6 +1,7 @@ import * as z from "zod/v4"; import { logEvent, commandPreview } from "../logger.js"; import type { ServerConfig } from "../config.js"; +import type { Workspace, WorkspaceRegistry } from "../workspaces.js"; import { WORKSPACE_APP_URI, type DiffStats, @@ -107,6 +108,52 @@ export function textBlock(text: string): ToolContent { return { type: "text", text }; } +const SKILL_URI_SHELL_ARGUMENT = + /"(skills:\/\/[^"\r\n]+)"|'(skills:\/\/[^'\r\n]+)'|(?])(skills:\/\/[^\s"'\`|&;()<>]+)/g; + +export async function expandSkillUrisInShellCommand( + config: ServerConfig, + workspaces: WorkspaceRegistry, + workspace: Workspace, + command: string, + shell: "native" | "bash", +): Promise { + if (!config.experimentalSkillUris || !command.includes("skills://")) { + return command; + } + + // Experimental workaround: shell tools cannot consume skills:// URIs themselves. + // Rewrite only standalone URI arguments until MCP Skills/resources are broadly host-native. + const matches = Array.from(command.matchAll(SKILL_URI_SHELL_ARGUMENT)); + if (matches.length === 0) return command; + + let expanded = ""; + let offset = 0; + for (const match of matches) { + const index = match.index; + const uri = match[1] ?? match[2] ?? match[3]; + if (index === undefined || uri === undefined) continue; + + const resolved = await workspaces.resolveReadPath(workspace, uri); + expanded += command.slice(offset, index); + expanded += quoteShellPath(resolved.absolutePath, shell); + offset = index + match[0].length; + } + + return expanded + command.slice(offset); +} + +function quoteShellPath(path: string, shell: "native" | "bash"): string { + if (shell === "native" && process.platform === "win32") { + return `"${path}"`; + } + + const shellPath = process.platform === "win32" + ? path.replace(/\\/g, "/") + : path; + return `'${shellPath.replace(/'/g, `'\\''`)}'`; +} + export function countDiffStats(diff: string | undefined): DiffStats { if (!diff) return { additions: 0, removals: 0 }; diff --git a/src/workspaces.test.ts b/src/workspaces.test.ts index 914837502..7383f6c18 100644 --- a/src/workspaces.test.ts +++ b/src/workspaces.test.ts @@ -344,7 +344,7 @@ test("workspace cache evicts old contexts without losing advertised skill reads" ); await writeFile(resourceFile, "reference\n"); - const config = loadConfig(writeTestDevspaceConfig( + const configEnv = writeTestDevspaceConfig( join(context.root, ".bounded-home"), { server: { port: 1 }, @@ -355,14 +355,23 @@ test("workspace cache evicts old contexts without losing advertised skill reads" skills: { agentDir }, subagents: { enabled: true, instructions: "on-demand", providers: [] }, }, - )); + ); + const config = loadConfig({ + ...configEnv, + DEVSPACE_EXPERIMENTAL_SKILL_URIS: "1", + }); const store = new SqliteWorkspaceStore(stateDir); try { const registry = new WorkspaceRegistry(config, store); const first = await registry.openWorkspace(context.root); assert.equal( - (await registry.resolveReadPath(first.workspace, resourceFile)).absolutePath, + ( + await registry.resolveReadPath( + first.workspace, + "skills://cache-skill/reference.md", + ) + ).absolutePath, await realpath(resourceFile), ); @@ -373,7 +382,12 @@ test("workspace cache evicts old contexts without losing advertised skill reads" const restored = await registry.getWorkspace(first.workspace.id); assert.notEqual(restored, first.workspace); assert.equal( - (await registry.resolveReadPath(restored, resourceFile)).absolutePath, + ( + await registry.resolveReadPath( + restored, + "skills://cache-skill/reference.md", + ) + ).absolutePath, await realpath(resourceFile), ); } finally { diff --git a/src/workspaces.ts b/src/workspaces.ts index 385e47cf2..293e82c25 100644 --- a/src/workspaces.ts +++ b/src/workspaces.ts @@ -414,15 +414,30 @@ export class WorkspaceRegistry { } async resolveReadPath(workspace: Workspace, inputPath: string): Promise { + if (this.config.experimentalSkillUris) { + const skillRead = resolveSkillReadPath(workspace.skills, inputPath, true); + if (skillRead) { + return { + absolutePath: await resolveCanonicalAllowedPath( + skillRead.absolutePath, + workspace.root, + [skillRead.skill.baseDir], + ), + skillRead, + }; + } + + return { + absolutePath: await this.resolvePath(workspace, inputPath), + }; + } + try { return { absolutePath: await this.resolvePath(workspace, inputPath), }; } catch (workspaceError) { - const skillRead = resolveSkillReadPath( - workspace.skills, - inputPath, - ); + const skillRead = resolveSkillReadPath(workspace.skills, inputPath, false); if (!skillRead) throw workspaceError; return { diff --git a/test/package-install-smoke.test.ts b/test/package-install-smoke.test.ts index c53477fd8..b6be06600 100644 --- a/test/package-install-smoke.test.ts +++ b/test/package-install-smoke.test.ts @@ -30,7 +30,6 @@ function testPackedPackageLaunchers(): void { "--no-fund", "--no-package-lock", "--no-save", - "--omit=optional", join(root, archive), ], { cwd: installRoot,