diff --git a/.env.example b/.env.example index 9e1341ca..f6ec0ccd 100644 --- a/.env.example +++ b/.env.example @@ -15,7 +15,6 @@ DEVSPACE_ALLOWED_ROOTS=/home/waishnav/personal,/home/waishnav/work # Advanced escape hatch. `*` disables Host header allowlist protection. # DEVSPACE_ALLOWED_HOSTS=localhost,127.0.0.1,your-public-host.example.com DEVSPACE_TOOL_MODE=full -DEVSPACE_TOOL_NAMING=legacy # off | changes | full. Defaults to changes. # changes creates one aggregate review widget via review_changes instead of per-tool iframes. DEVSPACE_WIDGETS=changes diff --git a/README.md b/README.md index 3605847c..16c8817d 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,14 @@ Most users should connect through a public HTTPS tunnel: https://your-tunnel-host.example.com/mcp ``` +> [!NOTE] +> Using DevSpace as an MCP connector isn't against OpenAI's Usage Policies โ€” it's +> a standard custom App/connector setup, and writing or running code isn't a +> restricted use case. But your account is governed by your usage, not by +> DevSpace. Don't point it at anything that would violate your provider's terms. +> Used normally, you're fine. (Based on OpenAI's Usage Policies and Service Terms +> as of June 2026.) + ## What ChatGPT Can Do Once connected, ChatGPT can open one of your approved project folders as a diff --git a/docs/chatgpt-coding-workflow.md b/docs/chatgpt-coding-workflow.md index c5efc662..8f75d57b 100644 --- a/docs/chatgpt-coding-workflow.md +++ b/docs/chatgpt-coding-workflow.md @@ -79,11 +79,17 @@ new context during later tool calls. Skills are enabled by default for coding-agent workflows. -DevSpace discovers skills from: +DevSpace discovers standard Agent Skills from: -- `DEVSPACE_AGENT_DIR`, which defaults to `~/.codex` -- project `.pi/skills` -- optional paths from `DEVSPACE_SKILL_PATHS` +- `~/.agents/skills` +- project `.agents/skills` + +It also keeps compatibility with: + +- `DEVSPACE_AGENT_DIR/skills`, defaulting to `~/.codex/skills` +- additional paths from `DEVSPACE_SKILL_PATHS` + +Legacy project paths such as `.pi/skills` can be added through `DEVSPACE_SKILL_PATHS` when needed. When `open_workspace` returns matching skills, the model should read the advertised `SKILL.md` before following that skill. @@ -97,7 +103,7 @@ Set `DEVSPACE_SKILLS=0` to hide skills from workspace output. ## Tool Names -Short names are the default: +DevSpace exposes these tool names: - `open_workspace` - `read` @@ -109,15 +115,21 @@ By default, DevSpace also runs in `DEVSPACE_TOOL_MODE=minimal`, so dedicated `grep`, `glob`, and `ls` tools are hidden. Use `bash` with command-line tools such as `rg`, `find`, and `ls` for search and directory inspection. -Legacy names are available with `DEVSPACE_TOOL_NAMING=legacy`: +Use `DEVSPACE_TOOL_MODE=full` to restore dedicated search and directory tools. + +The experimental Codex-style surface is enabled with +`DEVSPACE_TOOL_MODE=codex`. It exposes: - `open_workspace` -- `read_file` -- `write_file` -- `edit_file` -- `run_shell` +- `read` +- `apply_patch` +- `exec_command` +- `write_stdin` -Use `DEVSPACE_TOOL_MODE=full` to restore dedicated search and directory tools. +In this mode, `write`, `edit`, `bash`, `grep`, `glob`, and `ls` are not +registered. `exec_command` returns a process session ID when a command is still +running after its yield window. Use `write_stdin` to poll it, send input, resize +a PTY, or send Ctrl-C. Set `tty: true` only for commands that need a terminal. ## Show Changes @@ -129,6 +141,11 @@ and shell tools. The aggregate `show_changes` tool is not exposed by default. Use `DEVSPACE_WIDGETS=off` to disable widget UI, or `DEVSPACE_WIDGETS=changes` to expose the aggregate show-changes flow. +When `show_changes` is exposed, models should call it exactly once after the +final file modification in any turn that changes files. The tool only requires +the `workspaceId`; DevSpace automatically compares against the last shown +checkpoint and advances that checkpoint after rendering the aggregate diff. + ## Shell Use The shell tool is for commands that belong in a terminal: diff --git a/docs/configuration.md b/docs/configuration.md index 756123fe..ae56c9f2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -59,19 +59,23 @@ MCP clients discover metadata from: ## Tool Modes -`DEVSPACE_TOOL_NAMING` controls tool names. +`DEVSPACE_TOOL_MODE` controls the tool surface. | Value | Behavior | | --- | --- | -| `short` | Default. Uses `read`, `edit`, `bash`, and related names. | -| `legacy` | Uses `read_file`, `edit_file`, `run_shell`, and related names. | +| `minimal` | Default. Exposes `open_workspace`, `read`, `write`, `edit`, and `bash`. Clients use `bash` with tools such as `rg`, `find`, and `ls` for inspection. | +| `full` | Exposes the minimal tools plus dedicated `grep`, `glob`, and `ls` tools. | +| `codex` | Experimental. Exposes `open_workspace`, `read`, `apply_patch`, `exec_command`, and `write_stdin`. Existing mutation and shell tools are hidden. | -`DEVSPACE_TOOL_MODE` controls the tool surface. +`DEVSPACE_MINIMAL_TOOLS` remains a backward-compatible alias when +`DEVSPACE_TOOL_MODE` is unset: `1` selects `minimal` and `0` selects `full`. +The `codex` mode must be selected through `DEVSPACE_TOOL_MODE` and always uses +its fixed short tool names regardless of `DEVSPACE_TOOL_NAMING`. -| Value | Behavior | -| --- | --- | -| `minimal` | Default. Disables dedicated search and list tools. Clients use the shell tool with `rg`, `grep`, `find`, `ls`, or `tree` for inspection. | -| `full` | Enables dedicated `grep`, `glob`, and `ls` tools. | +Codex-mode commands run without a PTY by default. Set `tty: true` on +`exec_command` for interactive terminal programs. PTY support uses the optional +`node-pty` dependency; `write_stdin` can send input, poll output, and resize PTY +sessions. ## Widgets @@ -88,13 +92,25 @@ MCP clients discover metadata from: | Variable | Purpose | | --- | --- | | `DEVSPACE_SKILLS` | Set to `0` to hide skills. Enabled by default. | -| `DEVSPACE_AGENT_DIR` | Defaults to `~/.codex`. | -| `DEVSPACE_SKILL_PATHS` | Optional comma-separated skill directories. | +| `DEVSPACE_AGENT_DIR` | Defaults to `~/.codex`; its `skills` child is loaded for compatibility. | +| `DEVSPACE_SKILL_PATHS` | Optional comma-separated additional skill directories. | + +DevSpace discovers standard Agent Skills from: + +- `~/.agents/skills` +- project `.agents/skills` + +It also keeps compatibility with: + +- `DEVSPACE_AGENT_DIR/skills`, defaulting to `~/.codex/skills` +- additional paths from `DEVSPACE_SKILL_PATHS` + +Legacy project paths such as `.pi/skills` can be added through `DEVSPACE_SKILL_PATHS` when needed. Example: ```bash -DEVSPACE_SKILL_PATHS="$HOME/.codex/skills,$HOME/.claude/skills" \ +DEVSPACE_SKILL_PATHS="$HOME/.claude/skills,$HOME/company/skills" \ npx @waishnav/devspace serve ``` @@ -131,7 +147,6 @@ DEVSPACE_ALLOWED_ROOTS="$HOME/personal,$HOME/work" \ DEVSPACE_PUBLIC_BASE_URL="https://devspace.example.com" \ DEVSPACE_WORKTREE_ROOT="$HOME/.devspace/worktrees" \ DEVSPACE_TOOL_MODE="minimal" \ -DEVSPACE_TOOL_NAMING="short" \ DEVSPACE_WIDGETS="full" \ npx @waishnav/devspace serve ``` diff --git a/docs/gotchas.md b/docs/gotchas.md index d2e50274..d8d47f50 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -214,11 +214,17 @@ Skills are enabled by default. Check: DEVSPACE_SKILLS=1 npx @waishnav/devspace serve ``` -DevSpace looks in: +DevSpace looks in standard Agent Skills locations: -- `DEVSPACE_AGENT_DIR`, defaulting to `~/.codex` -- project `.pi/skills` -- `DEVSPACE_SKILL_PATHS` +- `~/.agents/skills` +- project `.agents/skills` + +It also checks compatibility and custom paths: + +- `DEVSPACE_AGENT_DIR/skills`, defaulting to `~/.codex/skills` +- additional paths from `DEVSPACE_SKILL_PATHS` + +Legacy project paths such as `.pi/skills` can be added through `DEVSPACE_SKILL_PATHS` when needed. If a skill appears in `open_workspace`, the model must read that skill's `SKILL.md` before reading other files inside the skill directory. diff --git a/package-lock.json b/package-lock.json index 690ad496..6419cf27 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,13 @@ { "name": "@waishnav/devspace", - "version": "1.0.2", + "version": "1.0.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@waishnav/devspace", - "version": "1.0.2", + "version": "1.0.3", + "hasInstallScript": true, "license": "MIT", "dependencies": { "@clack/prompts": "^1.5.1", @@ -15,6 +16,7 @@ "@modelcontextprotocol/sdk": "^1.29.0", "@pierre/diffs": "^1.2.5", "better-sqlite3": "^12.10.0", + "diff": "^8.0.3", "drizzle-orm": "^0.45.2", "express": "^5.2.1", "react": "^19.2.6", @@ -39,6 +41,9 @@ }, "engines": { "node": ">=22.19 <27" + }, + "optionalDependencies": { + "node-pty": "^1.1.0" } }, "node_modules/@clack/core": { @@ -688,6 +693,9 @@ "cpu": [ "arm64" ], + "libc": [ + "glibc" + ], "license": "MIT", "optional": true, "os": [ @@ -704,6 +712,9 @@ "cpu": [ "arm64" ], + "libc": [ + "musl" + ], "license": "MIT", "optional": true, "os": [ @@ -720,6 +731,9 @@ "cpu": [ "riscv64" ], + "libc": [ + "glibc" + ], "license": "MIT", "optional": true, "os": [ @@ -736,6 +750,9 @@ "cpu": [ "x64" ], + "libc": [ + "glibc" + ], "license": "MIT", "optional": true, "os": [ @@ -752,6 +769,9 @@ "cpu": [ "x64" ], + "libc": [ + "musl" + ], "license": "MIT", "optional": true, "os": [ @@ -3207,7 +3227,7 @@ } }, "node_modules/call-bind-apply-helpers": { - "version": "1.0.2", + "version": "1.0.3", "resolved": "https://registry.npmjs.org/call-bind-apply-helpers/-/call-bind-apply-helpers-1.0.2.tgz", "integrity": "sha512-Sp1ablJ0ivDkSzjcaJdxEunN5/XvksFJ2sMBFfq6x0ryhQV/2b/KwFe21cMpmHtPOSij8K99/wSfoEuTObmuMQ==", "license": "MIT", @@ -4693,6 +4713,24 @@ "node": ">=10" } }, + "node_modules/node-addon-api": { + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-7.1.1.tgz", + "integrity": "sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==", + "license": "MIT", + "optional": true + }, + "node_modules/node-pty": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/node-pty/-/node-pty-1.1.0.tgz", + "integrity": "sha512-20JqtutY6JPXTUnL0ij1uad7Qe1baT46lyolh2sSENDd4sTzKZ4nmAFkeAARDKwmlLjPx6XKRlwRUxwjOy+lUg==", + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "dependencies": { + "node-addon-api": "^7.1.0" + } + }, "node_modules/object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", @@ -5256,7 +5294,7 @@ } }, "node_modules/side-channel-weakmap": { - "version": "1.0.2", + "version": "1.0.3", "resolved": "https://registry.npmjs.org/side-channel-weakmap/-/side-channel-weakmap-1.0.2.tgz", "integrity": "sha512-WPS/HvHQTYnHisLo9McqBHOJk2FkHO/tlpvldyrnem4aeQp4hai3gythswg6p01oSoTl58rcpiFAjF2br2Ak2A==", "license": "MIT", @@ -5619,7 +5657,7 @@ } }, "node_modules/util-deprecate": { - "version": "1.0.2", + "version": "1.0.3", "resolved": "https://registry.npmjs.org/util-deprecate/-/util-deprecate-1.0.2.tgz", "integrity": "sha512-EPD5q1uXyFxJpCrLnCc1nHnq3gOa6DZBocAIiI2TaSCA7VCJ1UJDMagCzIkXNsUYfD1daK//LTEQ8xiIbrHtcw==", "license": "MIT" @@ -5755,7 +5793,7 @@ } }, "node_modules/wrappy": { - "version": "1.0.2", + "version": "1.0.3", "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", "integrity": "sha512-l4Sp/DRseor9wL6EvV2+TuQn63dMkPjZ/sp9XkghTEbV9KlPS1xUsZ3u7/IQO4wxtcFB4bgpQPRcR3QCvezPcQ==", "license": "ISC" diff --git a/package.json b/package.json index 7962ae98..e16711b3 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@waishnav/devspace", - "version": "1.0.2", + "version": "1.0.3", "description": "Expose a secure local coding workspace through an MCP server.", "type": "module", "main": "dist/server.js", @@ -24,8 +24,9 @@ "build": "npm run clean && npm run build:app && tsc -p tsconfig.build.json", "build:app": "vite build", "dev": "node scripts/dev-server.mjs", + "postinstall": "node scripts/fix-node-pty-permissions.mjs", "start": "node dist/cli.js serve", - "test": "tsx src/config.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", + "test": "tsx src/config.test.ts && tsx src/ui/card-types.test.ts && tsx src/ui/patch-display.test.ts && tsx src/apply-patch.test.ts && tsx src/process-platform.test.ts && tsx src/process-sessions.test.ts && tsx src/roots.test.ts && tsx src/skills.test.ts && tsx src/workspaces.test.ts && tsx src/review-checkpoints.test.ts && tsx src/oauth-store.test.ts && tsx src/cli.test.ts", "typecheck": "tsc -p tsconfig.json --noEmit" }, "keywords": [], @@ -38,6 +39,7 @@ "@modelcontextprotocol/sdk": "^1.29.0", "@pierre/diffs": "^1.2.5", "better-sqlite3": "^12.10.0", + "diff": "^8.0.3", "drizzle-orm": "^0.45.2", "express": "^5.2.1", "react": "^19.2.6", @@ -61,5 +63,8 @@ "protobufjs": "7.6.4", "ws": "8.21.0", "undici": "8.5.0" + }, + "optionalDependencies": { + "node-pty": "^1.1.0" } } diff --git a/scripts/fix-node-pty-permissions.mjs b/scripts/fix-node-pty-permissions.mjs new file mode 100644 index 00000000..1990bf44 --- /dev/null +++ b/scripts/fix-node-pty-permissions.mjs @@ -0,0 +1,22 @@ +import { chmod } from "node:fs/promises"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +if (process.platform === "darwin") { + const projectRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); + for (const architecture of ["arm64", "x64"]) { + const helper = resolve( + projectRoot, + "node_modules", + "node-pty", + "prebuilds", + `darwin-${architecture}`, + "spawn-helper", + ); + try { + await chmod(helper, 0o755); + } catch (error) { + if (error.code !== "ENOENT") throw error; + } + } +} diff --git a/src/apply-patch.test.ts b/src/apply-patch.test.ts new file mode 100644 index 00000000..c6f08695 --- /dev/null +++ b/src/apply-patch.test.ts @@ -0,0 +1,308 @@ +import assert from "node:assert/strict"; +import { chmod, mkdtemp, readFile, stat, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { applyPatch, isSamePatchFile, parsePatch, replaceFile } from "./apply-patch.js"; + +const root = await mkdtemp(join(tmpdir(), "devspace-apply-patch-")); +const replacement = join(root, "replacement.txt"); +const replacementTemporary = join(root, "replacement.tmp"); +await writeFile(replacement, "old\n"); +await writeFile(replacementTemporary, "new\n"); +await replaceFile(replacementTemporary, replacement, true, "win32"); +assert.equal(await readFile(replacement, "utf8"), "new\n"); + +const sameIdentity = async (): Promise<{ dev: number; ino: number }> => ({ dev: 1, ino: 2 }); +const differentIdentity = async (path: string): Promise<{ dev: number; ino: number }> => ({ + dev: 1, + ino: path.endsWith("foo.txt") ? 3 : 2, +}); +assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/Foo.txt"), true); +assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/foo.txt", sameIdentity), true); +assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/bar.txt", sameIdentity), false); +assert.equal(await isSamePatchFile("/tmp/Foo.txt", "/tmp/foo.txt", differentIdentity), false); + +await writeFile(join(root, "alpha.txt"), "one\ntwo\nthree\n"); +await writeFile(join(root, "remove.txt"), "remove me\n"); +await writeFile(join(root, "windows.txt"), "first\r\nsecond\r\n"); + +const result = await applyPatch( + root, + `*** Begin Patch +*** Add File: nested/added.txt ++new ++file +*** Update File: alpha.txt +@@ + one +-two ++changed + three +*** Update File: windows.txt +@@ + first +-second ++updated +*** Delete File: remove.txt +*** End Patch`, +); + +assert.deepEqual(result.files, [ + { path: "nested/added.txt", operation: "add" }, + { path: "alpha.txt", operation: "update" }, + { path: "windows.txt", operation: "update" }, + { path: "remove.txt", operation: "delete" }, +]); +assert.equal(result.additions, 4); +assert.equal(result.removals, 3); +assert.match(result.patch, /diff --git a\/alpha\.txt b\/alpha\.txt/); +assert.match(result.patch, /-two\n\+changed/); +assert.equal(await readFile(join(root, "nested/added.txt"), "utf8"), "new\nfile\n"); +assert.equal(await readFile(join(root, "alpha.txt"), "utf8"), "one\nchanged\nthree\n"); +assert.equal(await readFile(join(root, "windows.txt"), "utf8"), "first\r\nupdated\r\n"); +await assert.rejects(readFile(join(root, "remove.txt"), "utf8"), /ENOENT/); + +if (process.platform !== "win32") await chmod(join(root, "alpha.txt"), 0o755); +const moveResult = await applyPatch( + root, + `*** Begin Patch +*** Update File: alpha.txt +*** Move to: moved/alpha.txt +@@ +-one ++ONE + changed +*** End Patch`, +); +assert.deepEqual(moveResult.files, [ + { path: "moved/alpha.txt", previousPath: "alpha.txt", operation: "move" }, +]); +assert.equal(await readFile(join(root, "moved/alpha.txt"), "utf8"), "ONE\nchanged\nthree\n"); +if (process.platform !== "win32") { + assert.notEqual((await stat(join(root, "moved/alpha.txt"))).mode & 0o111, 0); +} +await assert.rejects(readFile(join(root, "alpha.txt"), "utf8"), /ENOENT/); + +await assert.rejects( + applyPatch( + root, + `*** Begin Patch +*** Add File: ../escape.txt ++no +*** End Patch`, + ), + /path escapes the workspace/, +); + +const outside = await mkdtemp(join(tmpdir(), "devspace-apply-patch-outside-")); +await symlink(outside, join(root, "outside-link"), process.platform === "win32" ? "junction" : "dir"); +await assert.rejects( + applyPatch( + root, + `*** Begin Patch +*** Add File: outside-link/escape.txt ++no +*** End Patch`, + ), + /path resolves outside the workspace/, +); + +await assert.rejects( + applyPatch( + root, + `*** Begin Patch +*** Update File: moved/alpha.txt +@@ +-not present ++replacement +*** End Patch`, + ), + /could not find hunk context/, +); +assert.equal(await readFile(join(root, "moved/alpha.txt"), "utf8"), "ONE\nchanged\nthree\n"); + +await assert.rejects( + applyPatch( + root, + `*** Begin Patch +*** Add File: should-not-exist.txt ++staged +*** Update File: moved/alpha.txt +@@ +-missing context ++replacement +*** End Patch`, + ), + /could not find hunk context/, +); +await assert.rejects(readFile(join(root, "should-not-exist.txt"), "utf8"), /ENOENT/); +assert.equal(await readFile(join(root, "moved/alpha.txt"), "utf8"), "ONE\nchanged\nthree\n"); + +const splitHunkRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-split-hunk-")); +await writeFile( + join(splitHunkRoot, "long.txt"), + Array.from({ length: 20 }, (_, index) => String(index + 1)).join("\n") + "\n", +); +const splitHunkResult = await applyPatch( + splitHunkRoot, + `*** Begin Patch +*** Update File: long.txt +@@ + 1 +-2 ++two + 3 +@@ + 17 +-18 ++eighteen + 19 +*** End Patch`, +); +assert.equal(splitHunkResult.patch.match(/^@@ /gm)?.length, 2); +assert.equal( + await readFile(join(splitHunkRoot, "long.txt"), "utf8"), + [ + "1", "two", "3", "4", "5", "6", "7", "8", "9", "10", + "11", "12", "13", "14", "15", "16", "17", "eighteen", "19", "20", + ].join("\n") + "\n", +); + +const trailingSpaceRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-trailing-space-")); +await writeFile(join(trailingSpaceRoot, "spaces.txt"), "old\n"); +const trailingSpaceResult = await applyPatch( + trailingSpaceRoot, + `*** Begin Patch +*** Update File: spaces.txt +@@ +-old ++new${" "} +*** End Patch`, +); +assert.equal(trailingSpaceResult.patch.endsWith("+new "), true); +assert.equal(await readFile(join(trailingSpaceRoot, "spaces.txt"), "utf8"), "new \n"); + +assert.throws(() => parsePatch("*** Begin Patch\n*** End Patch"), /contains no file actions/); +assert.throws(() => parsePatch("*** Add File: bad.txt\n+x"), /missing .* marker/); +assert.throws( + () => parsePatch("*** Begin Patch\n*** Add File: empty.txt\n*** End Patch"), + /has no content/, +); + +const overwriteRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-overwrite-")); +await writeFile(join(overwriteRoot, "duplicate.txt"), "old content\n"); +await applyPatch( + overwriteRoot, + `*** Begin Patch +*** Add File: duplicate.txt ++new content +*** End Patch`, +); +assert.equal(await readFile(join(overwriteRoot, "duplicate.txt"), "utf8"), "new content\n"); + +await writeFile(join(overwriteRoot, "source.txt"), "from\n"); +await writeFile(join(overwriteRoot, "destination.txt"), "existing\n"); +await applyPatch( + overwriteRoot, + `*** Begin Patch +*** Update File: source.txt +*** Move to: destination.txt +@@ +-from ++new +*** End Patch`, +); +assert.equal(await readFile(join(overwriteRoot, "destination.txt"), "utf8"), "new\n"); +await assert.rejects(readFile(join(overwriteRoot, "source.txt"), "utf8"), /ENOENT/); + +const noNewlineRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-newline-")); +await writeFile(join(noNewlineRoot, "no-newline.txt"), "old"); +await applyPatch( + noNewlineRoot, + `*** Begin Patch +*** Update File: no-newline.txt +@@ +-old ++new +*** End Patch`, +); +assert.equal(await readFile(join(noNewlineRoot, "no-newline.txt"), "utf8"), "new\n"); + +const eofRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-eof-")); +await writeFile(join(eofRoot, "tail.txt"), "first\nsecond\n"); +await applyPatch( + eofRoot, + `*** Begin Patch +*** Update File: tail.txt +@@ + first +-second ++second updated +*** End of File +*** End Patch`, +); +assert.equal(await readFile(join(eofRoot, "tail.txt"), "utf8"), "first\nsecond updated\n"); +await assert.rejects( + applyPatch( + eofRoot, + `*** Begin Patch +*** Update File: tail.txt +@@ + first ++not tail +*** End of File +*** End Patch`, + ), + /could not find hunk context/, +); + +const lenientRoot = await mkdtemp(join(tmpdir(), "devspace-apply-patch-lenient-")); +await writeFile(join(lenientRoot, "file.txt"), "one\n"); +await applyPatch( + lenientRoot, + `<<'EOF' + *** Begin Patch + *** Update File: file.txt +@@ +-one ++two + *** End Patch +EOF`, +); +assert.equal(await readFile(join(lenientRoot, "file.txt"), "utf8"), "two\n"); + +await applyPatch( + lenientRoot, + `*** Begin Patch +*** Environment ID: ignored +*** Update File: file.txt + two ++three +*** End Patch`, +); +assert.equal(await readFile(join(lenientRoot, "file.txt"), "utf8"), "two\nthree\n"); + +await assert.rejects( + applyPatch( + lenientRoot, + `*** Begin Patch +*** Add File: ${join(lenientRoot, "absolute.txt")} ++no +*** End Patch`, + ), + /path must be relative/, +); + +await writeFile(join(lenientRoot, "binary.dat"), Buffer.from([0, 159, 146, 150])); +await assert.rejects( + applyPatch( + lenientRoot, + `*** Begin Patch +*** Update File: binary.dat +@@ +-x ++y +*** End Patch`, + ), + /not valid UTF-8|binary/, +); diff --git a/src/apply-patch.ts b/src/apply-patch.ts new file mode 100644 index 00000000..32ec4a46 --- /dev/null +++ b/src/apply-patch.ts @@ -0,0 +1,485 @@ +import { randomUUID } from "node:crypto"; +import { constants, type Stats } from "node:fs"; +import { access, lstat, mkdir, readFile, realpath, rename, rm, stat, writeFile } from "node:fs/promises"; +import { dirname, isAbsolute, relative, resolve } from "node:path"; +import { TextDecoder } from "node:util"; +import { createTwoFilesPatch, FILE_HEADERS_ONLY } from "diff"; + +export type PatchOperation = "add" | "update" | "delete" | "move"; + +export interface AppliedPatchFile { + path: string; + previousPath?: string; + operation: PatchOperation; +} + +export interface ApplyPatchResult { + files: AppliedPatchFile[]; + patch: string; + additions: number; + removals: number; +} + +interface HunkLine { + kind: "context" | "add" | "remove"; + text: string; +} + +interface UpdateHunk { + lines: HunkLine[]; + changeContext?: string; + endOfFile?: boolean; +} + +type PatchAction = + | { kind: "add"; path: string; content: string } + | { kind: "delete"; path: string } + | { kind: "update"; path: string; moveTo?: string; hunks: UpdateHunk[] }; + +interface TextFile { + content: string; + mode?: number; +} + +type StagedTextFile = TextFile | null; +type FileIdentity = Pick; +type FileIdentityReader = (path: string) => Promise; + +function patchError(message: string): Error { + return new Error(`Invalid patch: ${message}`); +} + +export function parsePatch(patch: string): PatchAction[] { + const lines = patchLines(patch); + if (lines.shift()?.trim() !== "*** Begin Patch") { + throw patchError("missing *** Begin Patch marker"); + } + if (lines.pop()?.trim() !== "*** End Patch") { + throw patchError("missing *** End Patch marker"); + } + + const actions: PatchAction[] = []; + let index = 0; + + while (index < lines.length) { + const header = lines[index++].trim(); + if (header === "") continue; + + if (header.startsWith("*** Environment ID: ")) { + if (!header.slice("*** Environment ID: ".length).trim()) { + throw patchError("environment id cannot be empty"); + } + continue; + } + + if (header.startsWith("*** Add File: ")) { + const path = header.slice("*** Add File: ".length); + const content: string[] = []; + while (index < lines.length && !isTopLevelHeader(lines[index])) { + const line = lines[index++]; + if (!line.startsWith("+")) { + throw patchError(`added file line must start with +: ${line}`); + } + content.push(line.slice(1)); + } + if (content.length === 0) throw patchError(`add file for ${path} has no content`); + actions.push({ + kind: "add", + path, + content: `${content.join("\n")}\n`, + }); + continue; + } + + if (header.startsWith("*** Delete File: ")) { + actions.push({ kind: "delete", path: header.slice("*** Delete File: ".length) }); + continue; + } + + if (header.startsWith("*** Update File: ")) { + const path = header.slice("*** Update File: ".length); + let moveTo: string | undefined; + const hunks: UpdateHunk[] = []; + + if (lines[index]?.trim().startsWith("*** Move to: ")) { + moveTo = lines[index++].trim().slice("*** Move to: ".length); + } + + let current: UpdateHunk | undefined; + const finishCurrent = (): void => { + if (!current) return; + if (current.lines.length === 0) throw patchError(`empty update hunk for ${path}`); + hunks.push(current); + current = undefined; + }; + + while (index < lines.length) { + const line = lines[index]; + const trimmed = line.trim(); + if (!current && trimmed === "") { + index++; + continue; + } + if (trimmed === "*** End of File") { + if (!current) throw patchError(`end-of-file marker without update hunk for ${path}`); + current.endOfFile = true; + index++; + continue; + } + + if ((!current || !line.startsWith(" ")) && isTopLevelHeader(line)) break; + + if (trimmed.startsWith("@@") && !line.startsWith(" ")) { + finishCurrent(); + const changeContext = trimmed.slice(2).trim(); + current = { lines: [], changeContext: changeContext || undefined }; + index++; + continue; + } + + current ??= { lines: [] }; + index++; + if (line.startsWith(" ")) current.lines.push({ kind: "context", text: line.slice(1) }); + else if (line.startsWith("+")) current.lines.push({ kind: "add", text: line.slice(1) }); + else if (line.startsWith("-")) current.lines.push({ kind: "remove", text: line.slice(1) }); + else if (line === "\\ No newline at end of file") continue; + else throw patchError(`hunk line must start with space, +, or -: ${line}`); + } + finishCurrent(); + + if (hunks.length === 0 && !moveTo) { + throw patchError(`update for ${path} has no hunks or move destination`); + } + actions.push({ kind: "update", path, moveTo, hunks }); + continue; + } + + throw patchError(`unknown action header: ${header}`); + } + + if (actions.length === 0) throw patchError("contains no file actions"); + return actions; +} + +function patchLines(patch: string): string[] { + let lines = patch.replace(/\r\n/g, "\n").trim().split("\n"); + const first = lines[0]?.trim(); + const last = lines.at(-1)?.trim(); + if ( + (first === "<= 4 + ) { + lines = lines.slice(1, -1); + } + return lines; +} + +function isTopLevelHeader(line: string): boolean { + const trimmed = line.trim(); + return ( + trimmed.startsWith("*** Add File: ") || + trimmed.startsWith("*** Delete File: ") || + trimmed.startsWith("*** Update File: ") || + trimmed.startsWith("*** Environment ID: ") + ); +} + +function isInside(root: string, path: string): boolean { + const rel = relative(root, path); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); +} + +async function resolveConfinedPath(root: string, input: string): Promise { + if (!input || input.includes("\0") || isAbsolute(input)) { + throw patchError(`path must be relative to the workspace: ${input}`); + } + + const rootPath = await realpath(root); + const target = resolve(rootPath, input); + if (!isInside(rootPath, target)) { + throw patchError(`path escapes the workspace: ${input}`); + } + + let existing = target; + while (true) { + try { + const resolved = await realpath(existing); + if (!isInside(rootPath, resolved)) { + throw patchError(`path resolves outside the workspace: ${input}`); + } + break; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") throw error; + const parent = dirname(existing); + if (parent === existing) throw error; + existing = parent; + } + } + + return target; +} + +function splitFile(content: string): { lines: string[]; eol: string; finalNewline: boolean } { + const eol = content.includes("\r\n") ? "\r\n" : "\n"; + const normalized = content.replace(/\r\n/g, "\n"); + const finalNewline = normalized.endsWith("\n"); + const lines = normalized.split("\n"); + if (finalNewline) lines.pop(); + return { lines, eol, finalNewline }; +} + +function findSequence(haystack: string[], needle: string[], from: number, endOfFile = false): number { + if (needle.length === 0) return from; + + const matchAt = (index: number, normalize: (value: string) => string): boolean => + needle.every((line, offset) => normalize(haystack[index + offset] ?? "") === normalize(line)); + + for (const normalize of [ + (value: string) => value, + (value: string) => value.trimEnd(), + (value: string) => value.trim(), + ]) { + const start = endOfFile ? haystack.length - needle.length : from; + const end = haystack.length - needle.length; + for (let index = start; index <= end; index += 1) { + if (index >= from && matchAt(index, normalize)) return index; + } + } + + return -1; +} + +function applyHunks(path: string, content: string, hunks: UpdateHunk[]): string { + const file = splitFile(content); + const lines = [...file.lines]; + let cursor = 0; + + for (const hunk of hunks) { + if (hunk.changeContext) { + const contextIndex = findSequence(lines, [hunk.changeContext], cursor); + if (contextIndex < 0) { + throw patchError(`could not find hunk context in ${path}: ${hunk.changeContext}`); + } + cursor = contextIndex + 1; + } + + const oldLines = hunk.lines + .filter((line) => line.kind !== "add") + .map((line) => line.text); + const newLines = hunk.lines + .filter((line) => line.kind !== "remove") + .map((line) => line.text); + const index = hunk.endOfFile && oldLines.length === 0 + ? lines.length + : findSequence(lines, oldLines, cursor, hunk.endOfFile); + + if (index < 0) { + const preview = oldLines.slice(0, 3).join("\n"); + throw patchError(`could not find hunk context in ${path}: ${preview}`); + } + + lines.splice(index, oldLines.length, ...newLines); + cursor = index + newLines.length; + } + + const normalized = `${lines.join("\n")}\n`; + return file.eol === "\r\n" ? normalized.replace(/\n/g, "\r\n") : normalized; +} + +async function fileExists(path: string): Promise { + try { + await access(path, constants.F_OK); + return true; + } catch { + return false; + } +} + +export async function replaceFile( + temporary: string, + destination: string, + destinationExists: boolean, + platform: NodeJS.Platform = process.platform, +): Promise { + if (platform !== "win32" || !destinationExists) { + await rename(temporary, destination); + return; + } + + const backup = `${temporary}.original`; + await rename(destination, backup); + try { + await rename(temporary, destination); + } catch (error) { + await rename(backup, destination); + throw error; + } + await rm(backup, { force: true }); +} + +export async function isSamePatchFile( + source: string, + destination: string, + readIdentity: FileIdentityReader = lstat, +): Promise { + if (source === destination) return true; + if (source.toLowerCase() !== destination.toLowerCase()) return false; + + try { + const [sourceIdentity, destinationIdentity] = await Promise.all([ + readIdentity(source), + readIdentity(destination), + ]); + return sourceIdentity.dev === destinationIdentity.dev && sourceIdentity.ino === destinationIdentity.ino; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT" || code === "ENOTDIR") return false; + throw error; + } +} + +export async function applyPatch(root: string, patch: string): Promise { + const actions = parsePatch(patch); + const results: AppliedPatchFile[] = []; + const patches: string[] = []; + const staged = new Map(); + + const readStagedOptional = async (absolute: string, displayPath: string): Promise => { + if (staged.has(absolute)) return staged.get(absolute) ?? null; + const file = await readOptionalTextFile(absolute, displayPath); + staged.set(absolute, file); + return file; + }; + + const readStagedRequired = async (absolute: string, displayPath: string): Promise => { + const file = await readStagedOptional(absolute, displayPath); + if (!file) throw patchError(`file does not exist: ${displayPath}`); + return file; + }; + + for (const action of actions) { + if (action.kind === "add") { + const absolute = await resolveConfinedPath(root, action.path); + const original = await readStagedOptional(absolute, action.path); + staged.set(absolute, { content: action.content, mode: original?.mode }); + patches.push(unifiedFilePatch(action.path, action.path, original?.content ?? null, action.content)); + results.push({ path: action.path, operation: "add" }); + continue; + } + + const absolute = await resolveConfinedPath(root, action.path); + const file = await readStagedRequired(absolute, action.path); + + if (action.kind === "delete") { + staged.set(absolute, null); + patches.push(unifiedFilePatch(action.path, action.path, file.content, null)); + results.push({ path: action.path, operation: "delete" }); + continue; + } + + const updated = applyHunks(action.path, file.content, action.hunks); + if (action.moveTo) { + const destination = await resolveConfinedPath(root, action.moveTo); + const samePatchFile = await isSamePatchFile(absolute, destination); + if (!samePatchFile) await readStagedOptional(destination, action.moveTo); + if (samePatchFile) staged.delete(absolute); + staged.set(destination, { content: updated, mode: file.mode }); + if (!samePatchFile) staged.set(absolute, null); + patches.push(unifiedFilePatch(action.path, action.moveTo, file.content, updated)); + results.push({ path: action.moveTo, previousPath: action.path, operation: "move" }); + } else { + staged.set(absolute, { content: updated, mode: file.mode }); + patches.push(unifiedFilePatch(action.path, action.path, file.content, updated)); + results.push({ path: action.path, operation: "update" }); + } + } + + for (const [absolute, file] of staged) { + if (file) await writeTextFile(absolute, file.content, file.mode); + } + + for (const [absolute, file] of staged) { + if (!file) await rm(absolute, { force: true }); + } + + const unifiedPatch = patches.filter(Boolean).join("\n"); + const stats = countPatchStats(unifiedPatch); + return { files: results, patch: unifiedPatch, ...stats }; +} + +async function readOptionalTextFile(absolute: string, displayPath: string): Promise { + if (!(await fileExists(absolute))) return null; + const metadata = await stat(absolute); + if (!metadata.isFile()) throw patchError(`path is not a regular file: ${displayPath}`); + return { content: await readUtf8Text(absolute, displayPath), mode: metadata.mode }; +} + +async function readUtf8Text(absolute: string, displayPath: string): Promise { + const bytes = await readFile(absolute); + let content: string; + try { + content = new TextDecoder("utf-8", { fatal: true }).decode(bytes); + } catch { + throw patchError(`file is not valid UTF-8 text: ${displayPath}`); + } + if (content.includes("\0")) throw patchError(`file appears to be binary: ${displayPath}`); + return content; +} + +async function writeTextFile(destination: string, content: string, mode?: number): Promise { + await mkdir(dirname(destination), { recursive: true }); + const temporary = `${destination}.devspace-patch-${process.pid}-${randomUUID()}`; + try { + await writeFile(temporary, content, mode === undefined ? undefined : { mode }); + await replaceFile(temporary, destination, await fileExists(destination)); + } catch (error) { + await rm(temporary, { force: true }); + throw error; + } +} + +function unifiedFilePatch( + oldPath: string, + newPath: string, + oldContent: string | null, + newContent: string | null, +): string { + const oldFileName = oldContent === null ? "/dev/null" : `a/${oldPath}`; + const newFileName = newContent === null ? "/dev/null" : `b/${newPath}`; + const body = createTwoFilesPatch( + oldFileName, + newFileName, + oldContent ?? "", + newContent ?? "", + "", + "", + { context: 3, headerOptions: FILE_HEADERS_ONLY }, + ); + + return [ + `diff --git a/${oldPath} b/${newPath}`, + oldContent === null ? "new file mode 100644" : undefined, + newContent === null ? "deleted file mode 100644" : undefined, + stripFinalNewline(body), + ] + .filter((line): line is string => line !== undefined) + .join("\n"); +} + +function stripFinalNewline(value: string): string { + if (value.endsWith("\r\n")) return value.slice(0, -2); + if (value.endsWith("\n")) return value.slice(0, -1); + return value; +} + +function countPatchStats(patch: string): { additions: number; removals: number } { + let additions = 0; + let removals = 0; + for (const line of patch.split("\n")) { + if (line.startsWith("+") && !line.startsWith("+++")) additions += 1; + if (line.startsWith("-") && !line.startsWith("---")) removals += 1; + } + return { additions, removals }; +} diff --git a/src/config.test.ts b/src/config.test.ts index 38f5f6c2..ab5827bd 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -15,14 +15,12 @@ assert.equal(loadConfig(baseEnv).widgets, "full"); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_WIDGETS: "changes" }).widgets, "changes"); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_WIDGETS: "full" }).widgets, "full"); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_WIDGETS: "off" }).widgets, "off"); -assert.equal(loadConfig(baseEnv).toolNaming, "short"); -assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_NAMING: "short" }).toolNaming, "short"); -assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_NAMING: "legacy" }).toolNaming, "legacy"); -assert.equal(loadConfig(baseEnv).minimalTools, true); -assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "minimal" }).minimalTools, true); -assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "full" }).minimalTools, false); -assert.equal(loadConfig({ ...baseEnv, DEVSPACE_MINIMAL_TOOLS: "0" }).minimalTools, false); -assert.equal(loadConfig({ ...baseEnv, DEVSPACE_MINIMAL_TOOLS: "1" }).minimalTools, true); +assert.equal(loadConfig(baseEnv).toolMode, "minimal"); +assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "minimal" }).toolMode, "minimal"); +assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "full" }).toolMode, "full"); +assert.equal(loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "codex" }).toolMode, "codex"); +assert.equal(loadConfig({ ...baseEnv, DEVSPACE_MINIMAL_TOOLS: "0" }).toolMode, "full"); +assert.equal(loadConfig({ ...baseEnv, DEVSPACE_MINIMAL_TOOLS: "1" }).toolMode, "minimal"); assert.equal(loadConfig(baseEnv).skillsEnabled, true); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_SKILLS: "0" }).skillsEnabled, false); assert.equal(loadConfig({ ...baseEnv, DEVSPACE_SKILLS: "1" }).skillsEnabled, true); @@ -43,10 +41,6 @@ assert.throws( () => loadConfig({ ...baseEnv, DEVSPACE_TOOL_MODE: "invalid" }), /Invalid DEVSPACE_TOOL_MODE: invalid/, ); -assert.throws( - () => loadConfig({ ...baseEnv, DEVSPACE_TOOL_NAMING: "invalid" }), - /Invalid DEVSPACE_TOOL_NAMING: invalid/, -); assert.deepEqual(loadConfig(baseEnv).logging, { level: "info", diff --git a/src/config.ts b/src/config.ts index 805de8eb..b700ef51 100644 --- a/src/config.ts +++ b/src/config.ts @@ -5,7 +5,7 @@ import type { LoggingConfig, LogFormat, LogLevel } from "./logger.js"; import type { OAuthConfig } from "./oauth-provider.js"; import { loadDevspaceFiles } from "./user-config.js"; -export type ToolNamingMode = "legacy" | "short"; +export type ToolMode = "minimal" | "full" | "codex"; export type WidgetMode = "off" | "changes" | "full"; const DEFAULT_OAUTH_ACCESS_TOKEN_TTL_SECONDS = 60 * 60; const DEFAULT_OAUTH_REFRESH_TOKEN_TTL_SECONDS = 30 * 24 * 60 * 60; @@ -17,8 +17,7 @@ export interface ServerConfig { allowedRoots: string[]; allowedHosts: string[]; publicBaseUrl: string; - minimalTools: boolean; - toolNaming: ToolNamingMode; + toolMode: ToolMode; widgets: WidgetMode; stateDir: string; worktreeRoot: string; @@ -134,14 +133,15 @@ function resolveTrustProxyHops( return raw; } -function parseMinimalTools(env: NodeJS.ProcessEnv): boolean { - if (env.DEVSPACE_TOOL_MODE === "minimal") return true; - if (env.DEVSPACE_TOOL_MODE === "full") return false; - if (env.DEVSPACE_TOOL_MODE) { - throw new Error(`Invalid DEVSPACE_TOOL_MODE: ${env.DEVSPACE_TOOL_MODE}`); +function parseToolMode(env: NodeJS.ProcessEnv): ToolMode { + const mode = env.DEVSPACE_TOOL_MODE; + if (mode === "minimal" || mode === "full" || mode === "codex") return mode; + if (mode) throw new Error(`Invalid DEVSPACE_TOOL_MODE: ${mode}`); + + if (env.DEVSPACE_MINIMAL_TOOLS !== undefined) { + return parseBoolean(env.DEVSPACE_MINIMAL_TOOLS) ? "minimal" : "full"; } - if (env.DEVSPACE_MINIMAL_TOOLS !== undefined) return parseBoolean(env.DEVSPACE_MINIMAL_TOOLS); - return true; + return "minimal"; } function parseLogLevel(value: string | undefined): LogLevel { @@ -163,8 +163,7 @@ function parsePathList(value: string | undefined): string[] { value ?.split(",") .map((entry) => entry.trim()) - .filter(Boolean) - .map((entry) => resolve(expandHomePath(entry))) ?? [] + .filter(Boolean) ?? [] ); } @@ -188,13 +187,6 @@ function parsePositiveInteger(value: string | undefined, fallback: number, name: return parsed; } -function parseToolNaming(value: string | undefined): ToolNamingMode { - if (!value || value === "short") return "short"; - if (value === "legacy") return "legacy"; - - throw new Error(`Invalid DEVSPACE_TOOL_NAMING: ${value}`); -} - function parseLoggingConfig(env: NodeJS.ProcessEnv): LoggingConfig { return { level: parseLogLevel(env.DEVSPACE_LOG_LEVEL), @@ -282,8 +274,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): ServerConfig { allowedRoots: parseAllowedRoots(env.DEVSPACE_ALLOWED_ROOTS ?? files.config.allowedRoots), allowedHosts: parseAllowedHosts(env.DEVSPACE_ALLOWED_HOSTS, derivedAllowedHosts), publicBaseUrl, - minimalTools: parseMinimalTools(env), - toolNaming: parseToolNaming(env.DEVSPACE_TOOL_NAMING), + toolMode: parseToolMode(env), widgets: parseWidgetMode(env.DEVSPACE_WIDGETS), stateDir: resolve(expandHomePath(env.DEVSPACE_STATE_DIR ?? files.config.stateDir ?? defaultStateDir())), worktreeRoot: resolve(expandHomePath(env.DEVSPACE_WORKTREE_ROOT ?? files.config.worktreeRoot ?? defaultWorktreeRoot())), diff --git a/src/process-platform.test.ts b/src/process-platform.test.ts new file mode 100644 index 00000000..39b494d8 --- /dev/null +++ b/src/process-platform.test.ts @@ -0,0 +1,61 @@ +import assert from "node:assert/strict"; +import { resolveShellCommand, terminateProcessTree } from "./process-platform.js"; + +assert.deepEqual(resolveShellCommand("echo ok", "win32", { ComSpec: "C:\\Windows\\cmd.exe" }), { + executable: "C:\\Windows\\cmd.exe", + args: ["/d", "/s", "/c", "echo ok"], +}); + +assert.deepEqual(resolveShellCommand("echo ok", "darwin", { SHELL: "/bin/zsh" }), { + executable: "/bin/zsh", + args: ["-lc", "echo ok"], +}); + +assert.deepEqual(resolveShellCommand("echo ok", "linux", { SHELL: "/bin/dash" }), { + executable: "/bin/dash", + args: ["-c", "echo ok"], +}); + +assert.deepEqual(resolveShellCommand("echo ok", "linux", { SHELL: "/usr/bin/fish" }), { + executable: "/bin/sh", + args: ["-c", "echo ok"], +}); + +const windowsCalls: string[] = []; +terminateProcessTree( + { pid: 42, kill: (signal) => (windowsCalls.push(`child:${signal}`), true) }, + "SIGTERM", + false, + { + platform: "win32", + killGroup: () => undefined, + killWindowsTree: (pid) => (windowsCalls.push(`tree:${pid}`), true), + }, +); +assert.deepEqual(windowsCalls, ["tree:42"]); + +const posixCalls: string[] = []; +terminateProcessTree( + { pid: 43, kill: (signal) => (posixCalls.push(`child:${signal}`), true) }, + "SIGINT", + true, + { + platform: "darwin", + killGroup: (pid, signal) => posixCalls.push(`group:${pid}:${signal}`), + killWindowsTree: () => false, + }, +); +assert.deepEqual(posixCalls, ["group:43:SIGINT"]); + +const fallbackCalls: string[] = []; +terminateProcessTree( + { pid: 44, kill: (signal) => (fallbackCalls.push(`child:${signal}`), true) }, + "SIGTERM", + false, + { + platform: "linux", + killGroup: () => undefined, + killWindowsTree: () => false, + }, +); +assert.deepEqual(fallbackCalls, ["child:SIGTERM"]); diff --git a/src/process-platform.ts b/src/process-platform.ts new file mode 100644 index 00000000..905d4d73 --- /dev/null +++ b/src/process-platform.ts @@ -0,0 +1,77 @@ +import { basename } from "node:path"; +import { spawnSync } from "node:child_process"; + +export interface ShellCommand { + executable: string; + args: string[]; +} + +export interface KillableProcess { + pid?: number; + kill(signal?: NodeJS.Signals): boolean; +} + +interface ProcessTreeRuntime { + platform: NodeJS.Platform; + killGroup(pid: number, signal: NodeJS.Signals): void; + killWindowsTree(pid: number): boolean; +} + +const defaultProcessTreeRuntime: ProcessTreeRuntime = { + platform: process.platform, + killGroup: (pid, signal) => process.kill(-pid, signal), + killWindowsTree: (pid) => { + const result = spawnSync("taskkill.exe", ["/pid", String(pid), "/T", "/F"], { + stdio: "ignore", + windowsHide: true, + }); + return !result.error && result.status === 0; + }, +}; + +const LOGIN_SHELLS = new Set(["bash", "ksh", "zsh"]); +const POSIX_SHELLS = new Set(["ash", "dash", "sh"]); + +export function resolveShellCommand( + command: string, + platform: NodeJS.Platform = process.platform, + environment: NodeJS.ProcessEnv = process.env, +): ShellCommand { + if (platform === "win32") { + return { + executable: environment.ComSpec ?? environment.COMSPEC ?? "cmd.exe", + args: ["/d", "/s", "/c", command], + }; + } + + const configuredShell = environment.SHELL; + const shellName = configuredShell ? basename(configuredShell) : ""; + if (configuredShell && LOGIN_SHELLS.has(shellName)) { + return { executable: configuredShell, args: ["-lc", command] }; + } + if (configuredShell && POSIX_SHELLS.has(shellName)) { + return { executable: configuredShell, args: ["-c", command] }; + } + + return { executable: "/bin/sh", args: ["-c", command] }; +} + +export function terminateProcessTree( + child: KillableProcess, + signal: NodeJS.Signals, + detached: boolean, + runtime: ProcessTreeRuntime = defaultProcessTreeRuntime, +): void { + if (runtime.platform === "win32" && child.pid) { + if (runtime.killWindowsTree(child.pid)) return; + } else if (detached && child.pid) { + try { + runtime.killGroup(child.pid, signal); + return; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ESRCH") return; + } + } + + child.kill(signal); +} diff --git a/src/process-sessions.test.ts b/src/process-sessions.test.ts new file mode 100644 index 00000000..346c67cd --- /dev/null +++ b/src/process-sessions.test.ts @@ -0,0 +1,216 @@ +import assert from "node:assert/strict"; +import { HeadTailBuffer, ProcessSessionManager } from "./process-sessions.js"; + +const smallBuffer = new HeadTailBuffer(100); +smallBuffer.append("hello\n"); +assert.deepEqual(smallBuffer.drain(100), { output: "hello\n", truncated: false }); +assert.deepEqual(smallBuffer.drain(100), { output: "", truncated: false }); + +const headTail = new HeadTailBuffer(10); +headTail.append("start-middle-end"); +const headTailResult = headTail.drain(1_000); +assert.equal(headTailResult.truncated, true); +assert.match(headTailResult.output, /^start/); +assert.match(headTailResult.output, /e-end$/); +assert.match(headTailResult.output, /characters omitted/); + +const responseLimited = new HeadTailBuffer(100); +responseLimited.append("abcdef".repeat(20)); +const responseLimitedResult = responseLimited.drain(40); +assert.equal(responseLimitedResult.truncated, true); +assert.match(responseLimitedResult.output, /^abc/); +assert.match(responseLimitedResult.output, /def$/); + +const unicodeBuffer = new HeadTailBuffer(4); +unicodeBuffer.append("a๐Ÿ™‚b๐Ÿ™‚c"); +const unicodeResult = unicodeBuffer.drain(1_000); +assert.equal(unicodeResult.truncated, true); +assert.match(unicodeResult.output, /^a๐Ÿ™‚/); +assert.match(unicodeResult.output, /๐Ÿ™‚c$/); + +const manager = new ProcessSessionManager({ + maxBufferCharacters: 1_024, + completedSessionTtlMs: 1_000, +}); + +const node = process.platform === "win32" + ? `"${process.execPath}"` + : JSON.stringify(process.execPath); + +const foreground = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "console.log('foreground')"`, + yieldTimeMs: 2_000, +}); +assert.equal(foreground.running, false); +assert.equal(foreground.exitCode, 0); +assert.match(foreground.output, /foreground/); +assert.equal(foreground.sessionId, undefined); + +const environment = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "console.log([process.env.NO_COLOR, process.env.TERM, process.env.PAGER, process.env.GIT_PAGER, process.env.GH_PAGER, process.env.CODEX_CI].join(','))"`, + yieldTimeMs: 2_000, +}); +assert.equal(environment.running, false); +assert.match(environment.output, /1,dumb,cat,cat,cat,1/); + +const background = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "setTimeout(() => console.log('finished'), 100)"`, + yieldTimeMs: 5, +}); +assert.equal(background.running, true); +assert.ok(background.sessionId); +assert.equal(typeof background.sessionId, "number"); + +await assert.rejects( + manager.write({ + workspaceId: "workspace-b", + sessionId: background.sessionId, + yieldTimeMs: 1, + }), + /does not belong to workspace/, +); + +const completed = await manager.write({ + workspaceId: "workspace-a", + sessionId: background.sessionId, + yieldTimeMs: 2_000, +}); +assert.equal(completed.running, false); +assert.equal(completed.exitCode, 0); +assert.match(completed.output, /finished/); + +const interactive = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "process.stdin.once('data', data => { console.log('input:' + data.toString().trim()); process.exit(0); })"`, + yieldTimeMs: 5, +}); +assert.equal(interactive.running, true); +assert.ok(interactive.sessionId); +assert.equal(typeof interactive.sessionId, "number"); + +const inputResult = await manager.write({ + workspaceId: "workspace-a", + sessionId: interactive.sessionId, + chars: "hello\n", + yieldTimeMs: 2_000, +}); +assert.equal(inputResult.running, false); +assert.match(inputResult.output, /input:hello/); + +const defaultInteractive = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "process.stdin.once('data', data => setTimeout(() => { console.log('default-input:' + data.toString().trim()); process.exit(0); }, 100))"`, + yieldTimeMs: 5, +}); +assert.equal(defaultInteractive.running, true); +assert.ok(defaultInteractive.sessionId); + +const defaultInputResult = await manager.write({ + workspaceId: "workspace-a", + sessionId: defaultInteractive.sessionId, + chars: "hello\n", +}); +assert.equal(defaultInputResult.running, false); +assert.match(defaultInputResult.output, /default-input:hello/); + +const noisyInteractive = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "setInterval(() => console.log('tick'), 10); process.stdin.once('data', data => { console.log('input:' + data.toString().trim()); process.exit(0); })"`, + yieldTimeMs: 100, +}); +assert.equal(noisyInteractive.running, true); +assert.ok(noisyInteractive.sessionId); + +await new Promise((resolve) => setTimeout(resolve, 50)); +const noisyInputResult = await manager.write({ + workspaceId: "workspace-a", + sessionId: noisyInteractive.sessionId, + chars: "hello\n", + yieldTimeMs: 2_000, +}); +assert.equal(noisyInputResult.running, false); +assert.match(noisyInputResult.output, /input:hello/); + +const interruptible = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "setInterval(() => console.log('tick'), 10)"`, + yieldTimeMs: 100, +}); +assert.equal(interruptible.running, true); +assert.ok(interruptible.sessionId); + +await new Promise((resolve) => setTimeout(resolve, 50)); +const interrupted = await manager.write({ + workspaceId: "workspace-a", + sessionId: interruptible.sessionId, + chars: "\u0003", + yieldTimeMs: 2_000, +}); +assert.equal(interrupted.running, false); +if (process.platform !== "win32") assert.equal(interrupted.signal, "SIGINT"); + +let buffered = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "console.log('x'.repeat(5000)); setTimeout(() => {}, 100)"`, + yieldTimeMs: 50, + maxOutputTokens: 100, +}); +if (!buffered.outputTruncated && buffered.sessionId) { + buffered = await manager.write({ + workspaceId: "workspace-a", + sessionId: buffered.sessionId, + yieldTimeMs: 2_000, + maxOutputTokens: 100, + }); +} +assert.equal(buffered.outputTruncated, true); +if (buffered.sessionId) manager.terminate("workspace-a", buffered.sessionId); + +try { + if (process.platform === "win32") { + const pty = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: "echo pty-ok", + tty: true, + yieldTimeMs: 10_000, + }); + assert.equal(pty.running, false); + assert.match(pty.output, /pty-ok/); + } else { + const pty = await manager.start({ + workspaceId: "workspace-a", + cwd: process.cwd(), + command: `${node} -e "setTimeout(() => console.log('columns:' + process.stdout.columns), 250)"`, + tty: true, + columns: 80, + rows: 24, + yieldTimeMs: 10, + }); + assert.equal(pty.running, true); + assert.ok(pty.sessionId); + + const resizedPty = await manager.write({ + workspaceId: "workspace-a", + sessionId: pty.sessionId, + columns: 120, + rows: 30, + yieldTimeMs: 2_000, + }); + assert.equal(resizedPty.running, false); + assert.match(resizedPty.output, /columns:120/); + } +} finally { + manager.shutdown(); +} diff --git a/src/process-sessions.ts b/src/process-sessions.ts new file mode 100644 index 00000000..9884df12 --- /dev/null +++ b/src/process-sessions.ts @@ -0,0 +1,422 @@ +import { spawn } from "node:child_process"; +import { resolveShellCommand, terminateProcessTree } from "./process-platform.js"; + +const DEFAULT_EXEC_YIELD_MS = 10_000; +const DEFAULT_INTERACTIVE_YIELD_MS = 250; +const DEFAULT_POLL_YIELD_MS = 5_000; +const MAX_COMMAND_YIELD_MS = 30_000; +const MAX_POLL_YIELD_MS = 110_000; +const DEFAULT_MAX_OUTPUT_TOKENS = 10_000; +const DEFAULT_BUFFER_CHARACTERS = 1_000_000; +const COMPLETED_SESSION_TTL_MS = 5 * 60 * 1_000; +const DEFAULT_COLUMNS = 80; +const DEFAULT_ROWS = 24; + +export interface StartCommandInput { + workspaceId: string; + command: string; + cwd: string; + tty?: boolean; + columns?: number; + rows?: number; + yieldTimeMs?: number; + maxOutputTokens?: number; +} + +export interface WriteStdinInput { + workspaceId: string; + sessionId: number; + chars?: string; + columns?: number; + rows?: number; + yieldTimeMs?: number; + maxOutputTokens?: number; +} + +export interface ProcessSnapshot { + sessionId?: number; + output: string; + outputTruncated: boolean; + running: boolean; + exitCode?: number; + signal?: string; + wallTimeMs: number; +} + +interface ManagedProcess { + write(data: string): void; + kill(signal?: NodeJS.Signals): void; + resize?(columns: number, rows: number): void; +} + +interface ProcessSession { + id: number; + workspaceId: string; + process?: ManagedProcess; + startedAt: number; + columns: number; + rows: number; + buffer: HeadTailBuffer; + running: boolean; + exitCode?: number; + signal?: string; + exitPromise: Promise; + resolveExit: () => void; + cleanupTimer?: NodeJS.Timeout; +} + +interface ProcessSessionManagerOptions { + maxBufferCharacters?: number; + completedSessionTtlMs?: number; +} + +function boundedInteger(value: number | undefined, fallback: number, maximum: number): number { + if (value === undefined) return fallback; + if (!Number.isFinite(value) || value < 0) { + throw new Error("Duration and output limits must be non-negative."); + } + return Math.min(Math.floor(value), maximum); +} + +function terminalSize(value: number | undefined, fallback: number): number { + if (value === undefined) return fallback; + if (!Number.isInteger(value) || value < 1 || value > 1_000) { + throw new Error("Terminal dimensions must be integers between 1 and 1000."); + } + return value; +} + +function processEnvironment(): Record { + return { + ...Object.fromEntries( + Object.entries(process.env).filter((entry): entry is [string, string] => entry[1] !== undefined), + ), + NO_COLOR: "1", + TERM: "dumb", + PAGER: "cat", + GIT_PAGER: "cat", + GH_PAGER: "cat", + CODEX_CI: "1", + LANG: process.env.LANG ?? "C.UTF-8", + LC_ALL: process.env.LC_ALL ?? "C.UTF-8", + }; +} + +function codePointLength(value: string): number { + return Array.from(value).length; +} + +function sliceCodePoints(value: string, start: number, end?: number): string { + return Array.from(value).slice(start, end).join(""); +} + +function takeHead(value: string, count: number): string { + if (count <= 0) return ""; + return sliceCodePoints(value, 0, count); +} + +function takeTail(value: string, count: number): string { + if (count <= 0) return ""; + const characters = Array.from(value); + return characters.slice(Math.max(0, characters.length - count)).join(""); +} + +function splitBudget(maxCharacters: number): { head: number; tail: number } { + return { + head: Math.ceil(maxCharacters / 2), + tail: Math.floor(maxCharacters / 2), + }; +} + +function formatHeadTail(head: string, tail: string, omittedCharacters: number): string { + if (omittedCharacters <= 0) return head + tail; + return `${head}\n... output truncated (${omittedCharacters} characters omitted) ...\n${tail}`; +} + +export class HeadTailBuffer { + private head = ""; + private tail = ""; + private totalCharacters = 0; + + constructor(private readonly maxCharacters: number) { + if (!Number.isInteger(maxCharacters) || maxCharacters < 1) { + throw new Error("Head/tail buffer limit must be a positive integer."); + } + } + + append(output: string): void { + if (!output) return; + + const previousTotal = this.totalCharacters; + this.totalCharacters += codePointLength(output); + + if (this.totalCharacters <= this.maxCharacters) { + this.head += output; + return; + } + + const budget = splitBudget(this.maxCharacters); + if (previousTotal <= this.maxCharacters) { + const fullOutput = this.head + output; + this.head = takeHead(fullOutput, budget.head); + this.tail = takeTail(fullOutput, budget.tail); + return; + } + + this.tail = takeTail(this.tail + output, budget.tail); + } + + hasOutput(): boolean { + return this.totalCharacters > 0; + } + + drain(maxCharacters: number): { output: string; truncated: boolean } { + if (!Number.isInteger(maxCharacters) || maxCharacters < 1) { + throw new Error("Output limit must be a positive integer."); + } + + const omittedByBuffer = Math.max( + 0, + this.totalCharacters - codePointLength(this.head) - codePointLength(this.tail), + ); + const retained = formatHeadTail(this.head, this.tail, omittedByBuffer); + const output = truncateOutput(retained, maxCharacters); + const truncated = omittedByBuffer > 0 || output.truncated; + + this.head = ""; + this.tail = ""; + this.totalCharacters = 0; + + return { output: output.output, truncated }; + } +} + +function truncateOutput(output: string, maxCharacters: number): { output: string; truncated: boolean } { + const outputCharacters = codePointLength(output); + if (outputCharacters <= maxCharacters) return { output, truncated: false }; + + const marker = "\n... output truncated ...\n"; + const markerCharacters = codePointLength(marker); + const available = Math.max(0, maxCharacters - markerCharacters); + const budget = splitBudget(available); + return { + output: takeHead(output, budget.head) + marker + takeTail(output, budget.tail), + truncated: true, + }; +} + +export class ProcessSessionManager { + private readonly sessions = new Map(); + private readonly maxBufferCharacters: number; + private readonly completedSessionTtlMs: number; + private nextSessionId = 1; + + constructor(options: ProcessSessionManagerOptions = {}) { + this.maxBufferCharacters = options.maxBufferCharacters ?? DEFAULT_BUFFER_CHARACTERS; + this.completedSessionTtlMs = options.completedSessionTtlMs ?? COMPLETED_SESSION_TTL_MS; + } + + async start(input: StartCommandInput): Promise { + const session = this.createSession(input); + this.sessions.set(session.id, session); + + try { + if (input.tty && process.platform !== "win32") await this.startPty(session, input); + else this.startPipe(session, input); + } catch (error) { + this.sessions.delete(session.id); + throw error; + } + + const yieldTimeMs = boundedInteger(input.yieldTimeMs, DEFAULT_EXEC_YIELD_MS, MAX_COMMAND_YIELD_MS); + await this.waitForExit(session, yieldTimeMs); + + const snapshot = this.consume(session, input.maxOutputTokens); + if (!session.running) this.removeSession(session.id); + return snapshot; + } + + async write(input: WriteStdinInput): Promise { + const session = this.getOwnedSession(input.workspaceId, input.sessionId); + const chars = input.chars ?? ""; + const interactionRequested = + chars.length > 0 || input.columns !== undefined || input.rows !== undefined; + + if (input.columns !== undefined || input.rows !== undefined) { + session.columns = terminalSize(input.columns, session.columns); + session.rows = terminalSize(input.rows, session.rows); + if (!session.process?.resize) { + throw new Error(`Process session ${session.id} is not a PTY and cannot be resized.`); + } + session.process.resize(session.columns, session.rows); + } + + const interruptRequested = chars.includes("\u0003") && session.running; + if (interruptRequested) { + session.process?.kill("SIGINT"); + } + const writableChars = chars.replaceAll("\u0003", ""); + if (writableChars && session.running) session.process?.write(writableChars); + + if ((interactionRequested || !session.buffer.hasOutput()) && session.running) { + const fallback = interactionRequested ? DEFAULT_INTERACTIVE_YIELD_MS : DEFAULT_POLL_YIELD_MS; + const maximum = interactionRequested ? MAX_COMMAND_YIELD_MS : MAX_POLL_YIELD_MS; + const yieldTimeMs = boundedInteger(input.yieldTimeMs, fallback, maximum); + await this.waitForExit(session, yieldTimeMs); + } + + const snapshot = this.consume(session, input.maxOutputTokens); + if (!session.running) this.removeSession(session.id); + return snapshot; + } + + terminate(workspaceId: string, sessionId: number): void { + const session = this.getOwnedSession(workspaceId, sessionId); + if (session.running) session.process?.kill("SIGTERM"); + } + + shutdown(): void { + for (const session of this.sessions.values()) { + if (session.cleanupTimer) clearTimeout(session.cleanupTimer); + if (session.running) session.process?.kill("SIGTERM"); + } + this.sessions.clear(); + } + + private async waitForExit(session: ProcessSession, yieldTimeMs: number): Promise { + let timer: NodeJS.Timeout | undefined; + try { + await Promise.race([ + session.exitPromise, + new Promise((resolve) => { + timer = setTimeout(resolve, yieldTimeMs); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } + } + + private createSession(input: StartCommandInput): ProcessSession { + let resolveExit = (): void => undefined; + const exitPromise = new Promise((resolve) => { + resolveExit = resolve; + }); + + return { + id: this.nextSessionId++, + workspaceId: input.workspaceId, + startedAt: Date.now(), + columns: terminalSize(input.columns, DEFAULT_COLUMNS), + rows: terminalSize(input.rows, DEFAULT_ROWS), + buffer: new HeadTailBuffer(this.maxBufferCharacters), + running: true, + exitPromise, + resolveExit, + }; + } + + private startPipe(session: ProcessSession, input: StartCommandInput): void { + const shell = resolveShellCommand(input.command); + const detached = process.platform !== "win32"; + const child = spawn(input.command, { + cwd: input.cwd, + env: processEnvironment(), + stdio: "pipe", + windowsHide: true, + detached, + shell: shell.executable, + }); + + session.process = { + write: (data) => child.stdin.write(data), + kill: (signal = "SIGTERM") => terminateProcessTree(child, signal, detached), + resize: input.tty ? () => undefined : undefined, + }; + child.stdout.on("data", (data: Buffer) => this.append(session, data.toString("utf8"))); + child.stderr.on("data", (data: Buffer) => this.append(session, data.toString("utf8"))); + child.on("error", (error) => this.append(session, `${error.message}\n`)); + child.on("close", (code, signal) => this.finish(session, code ?? undefined, signal ?? undefined)); + } + + private async startPty(session: ProcessSession, input: StartCommandInput): Promise { + let nodePty: typeof import("node-pty"); + try { + nodePty = await import("node-pty"); + } catch { + throw new Error("PTY support requires the optional node-pty dependency."); + } + + const shell = resolveShellCommand(input.command); + let pty: import("node-pty").IPty; + try { + pty = nodePty.spawn(shell.executable, shell.args, { + cwd: input.cwd, + env: processEnvironment(), + name: "xterm-256color", + cols: session.columns, + rows: session.rows, + }); + } catch (error) { + throw error; + } + + session.process = { + write: (data) => pty.write(data), + kill: (signal) => pty.kill(signal), + resize: (columns, rows) => pty.resize(columns, rows), + }; + pty.onData((data) => this.append(session, data)); + pty.onExit(({ exitCode, signal }) => { + this.finish(session, exitCode, signal === 0 ? undefined : String(signal)); + }); + } + + private finish(session: ProcessSession, exitCode?: number, signal?: string): void { + if (!session.running) return; + session.running = false; + session.exitCode = exitCode; + session.signal = signal; + session.resolveExit(); + session.cleanupTimer = setTimeout( + () => this.sessions.delete(session.id), + this.completedSessionTtlMs, + ); + session.cleanupTimer.unref(); + } + + private append(session: ProcessSession, output: string): void { + session.buffer.append(output); + } + + private consume(session: ProcessSession, maxOutputTokens?: number): ProcessSnapshot { + const limit = boundedInteger(maxOutputTokens, DEFAULT_MAX_OUTPUT_TOKENS, 100_000); + const maxCharacters = Math.max(256, limit * 4); + const buffered = session.buffer.drain(maxCharacters); + + return { + sessionId: session.running ? session.id : undefined, + output: buffered.output, + outputTruncated: buffered.truncated, + running: session.running, + exitCode: session.exitCode, + signal: session.signal, + wallTimeMs: Date.now() - session.startedAt, + }; + } + + private getOwnedSession(workspaceId: string, sessionId: number): ProcessSession { + const session = this.sessions.get(sessionId); + if (!session) throw new Error(`Unknown process session: ${sessionId}`); + if (session.workspaceId !== workspaceId) { + throw new Error(`Process session ${sessionId} does not belong to workspace ${workspaceId}.`); + } + return session; + } + + private removeSession(sessionId: number): void { + const session = this.sessions.get(sessionId); + if (session?.cleanupTimer) clearTimeout(session.cleanupTimer); + this.sessions.delete(sessionId); + } +} diff --git a/src/roots.test.ts b/src/roots.test.ts index 96d479a0..90bdaa24 100644 --- a/src/roots.test.ts +++ b/src/roots.test.ts @@ -24,3 +24,10 @@ assert.equal( resolveAllowedPath("~/file.txt", "/workspace", ["/workspace"]), resolve("/workspace", "~/file.txt"), ); + +if (process.platform === "win32") { + assert.throws( + () => assertAllowedPath("C:\\Users\\Administrator", ["G:\\Projects\\Dev\\Github\\devspace"]), + /Path is outside allowed roots/, + ); +} diff --git a/src/roots.ts b/src/roots.ts index 0ebd49ab..214ffb2b 100644 --- a/src/roots.ts +++ b/src/roots.ts @@ -1,5 +1,5 @@ import { homedir } from "node:os"; -import { relative, resolve, sep } from "node:path"; +import { isAbsolute, relative, resolve, sep } from "node:path"; export class AccessDeniedError extends Error { constructor(message: string) { @@ -24,7 +24,10 @@ export function isPathInsideRoot(path: string, root: string): boolean { return ( relationship === "" || - (!relationship.startsWith("..") && relationship !== ".." && !relationship.includes(`..${sep}`)) + (!isAbsolute(relationship) && + !relationship.startsWith("..") && + relationship !== ".." && + !relationship.includes(`..${sep}`)) ); } diff --git a/src/server.ts b/src/server.ts index 752cf12d..ae905eda 100644 --- a/src/server.ts +++ b/src/server.ts @@ -17,6 +17,7 @@ import { import express from "express"; import type { Request, Response } from "express"; import * as z from "zod/v4"; +import { applyPatch } from "./apply-patch.js"; import { describeTrustProxy, loadConfig, @@ -41,6 +42,7 @@ import { writeFileTool, } from "./pi-tools.js"; import { SingleUserOAuthProvider } from "./oauth-provider.js"; +import { ProcessSessionManager, type ProcessSnapshot } from "./process-sessions.js"; import { createReviewCheckpointManager } from "./review-checkpoints.js"; import { formatPathForPrompt } from "./skills.js"; import { createWorkspaceStore } from "./workspace-store.js"; @@ -143,16 +145,16 @@ function toolWidgetDescriptorMeta( }; } -interface ToolNames { - openWorkspace: "open_workspace"; - read: "read_file" | "read"; - write: "write_file" | "write"; - edit: "edit_file" | "edit"; - grep: "grep_files" | "grep"; - glob: "find_files" | "glob"; - ls: "list_directory" | "ls"; - shell: "run_shell" | "bash"; -} +const toolNames = { + openWorkspace: "open_workspace", + read: "read", + write: "write", + edit: "edit", + grep: "grep", + glob: "glob", + ls: "ls", + shell: "bash", +} as const; interface ToolLogFields { tool: string; @@ -166,32 +168,17 @@ interface ToolLogFields { error?: string; } -function toolNamesFor(config: ServerConfig): ToolNames { - return config.toolNaming === "short" - ? { - openWorkspace: "open_workspace", - read: "read", - write: "write", - edit: "edit", - grep: "grep", - glob: "glob", - ls: "ls", - shell: "bash", - } - : { - openWorkspace: "open_workspace", - read: "read_file", - write: "write_file", - edit: "edit_file", - grep: "grep_files", - glob: "find_files", - ls: "list_directory", - shell: "run_shell", - }; -} +function serverInstructions(config: ServerConfig): string { + const showChangesInstruction = + config.widgets === "changes" + ? " If the turn successfully modifies files by creating, editing, overwriting, deleting, moving, or applying patches, call show_changes exactly once for that workspace after the final related file change and before your final response so the user can inspect the aggregate diff for that turn. Do not call it after every individual file change; do not skip it because individual file-change tools already returned diffs." + : ""; + + if (config.toolMode === "codex") { + return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree and reuse its workspaceId. Use ${toolNames.read} for direct file reads, apply_patch for all file modifications, exec_command for inspection, tests, builds, and other commands, and write_stdin to poll or interact with running processes. Follow instructions returned by ${toolNames.openWorkspace}; read applicable instruction and skill files before working in their scope.${showChangesInstruction}`; + } -function serverInstructions(config: ServerConfig, toolNames: ToolNames): string { - const inspection = config.minimalTools + const inspection = config.toolMode !== "full" ? `In minimal tool mode, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} are disabled; use ${toolNames.shell} with command-line tools such as grep, rg, find, ls, and tree for search and directory inspection. ` : `Prefer ${toolNames.read}, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} for file inspection. `; @@ -201,12 +188,7 @@ function serverInstructions(config: ServerConfig, toolNames: ToolNames): string const agentsMd = `Follow instructions returned by ${toolNames.openWorkspace}. Before working under a path listed in availableAgentsFiles, use ${toolNames.read} to inspect that instruction file and follow it. `; - const showChanges = - config.widgets === "changes" - ? " After creating, editing, or overwriting files, call show_changes once after the related file changes are complete so the user can see the aggregate diff." - : ""; - - return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree to obtain a workspaceId. Reuse that same workspaceId for all later file, search, edit, write, show-changes, and shell tools in that folder; do not call ${toolNames.openWorkspace} again unless switching folders/worktrees, changing checkout/worktree mode, the workspaceId is rejected as unknown, or the user explicitly asks to reopen. ${agentsMd}${skills}${inspection}Prefer ${toolNames.edit} for targeted modifications, ${toolNames.write} only for new files or complete rewrites, and ${toolNames.shell} for tests, builds, git inspection, package scripts, and commands that are better executed by the shell. Do not create or modify files with ${toolNames.shell}; avoid shell redirection, heredocs, tee, sed -i, perl -i, node/python/ruby scripts, or any command whose purpose is to write project files.${showChanges}`; + return `Use DevSpace as a local coding workspace. Call ${toolNames.openWorkspace} once per project folder or worktree to obtain a workspaceId. Reuse that same workspaceId for all later file, search, edit, write, show-changes, and shell tools in that folder; do not call ${toolNames.openWorkspace} again unless switching folders/worktrees, changing checkout/worktree mode, the workspaceId is rejected as unknown, or the user explicitly asks to reopen. ${agentsMd}${skills}${inspection}Prefer ${toolNames.edit} for targeted modifications, ${toolNames.write} only for new files or complete rewrites, and ${toolNames.shell} for tests, builds, git inspection, package scripts, and commands that are better executed by the shell. Do not create or modify files with ${toolNames.shell}; avoid shell redirection, heredocs, tee, sed -i, perl -i, node/python/ruby scripts, or any command whose purpose is to write project files.${showChangesInstruction}`; } function resultOutputSchema(extra: z.ZodRawShape = {}): z.ZodRawShape { return { @@ -458,12 +440,206 @@ async function assertWorkspaceAppAssets(): Promise { } } +function processResult(snapshot: ProcessSnapshot): string { + const status = snapshot.running + ? `Process running with session ID ${snapshot.sessionId}.` + : snapshot.signal + ? `Process exited after signal ${snapshot.signal}.` + : `Process exited with code ${snapshot.exitCode ?? "unknown"}.`; + return snapshot.output ? `${snapshot.output.replace(/\n$/, "")}\n${status}` : status; +} + +function processOutputSchema(): z.ZodRawShape { + return resultOutputSchema({ + sessionId: z.number().optional(), + running: z.boolean(), + exitCode: z.number().int().optional(), + signal: z.string().optional(), + wallTimeMs: z.number().nonnegative(), + outputTruncated: z.boolean(), + }); +} + +function processToolResponse( + tool: "exec_command" | "write_stdin", + workspaceId: string, + snapshot: ProcessSnapshot, + summary: Record, +) { + const result = processResult(snapshot); + const content = [textBlock(result)]; + const outputSummary = textSummary(snapshot.output ? [textBlock(snapshot.output)] : []); + return { + content, + _meta: { + tool, + card: { + workspaceId, + summary: { ...summary, ...outputSummary }, + payload: { content }, + }, + }, + structuredContent: { + result, + sessionId: snapshot.sessionId, + running: snapshot.running, + exitCode: snapshot.exitCode, + signal: snapshot.signal, + wallTimeMs: snapshot.wallTimeMs, + outputTruncated: snapshot.outputTruncated, + }, + }; +} + +function registerCodexProcessTools( + server: McpServer, + config: ServerConfig, + workspaces: WorkspaceRegistry, + processSessions: ProcessSessionManager, +): void { + registerAppTool( + server, + "exec_command", + { + title: "Execute command", + description: + "Run a command inside an open workspace. Returns its result when it exits during the yield window, otherwise returns a sessionId for write_stdin. Use this for file inspection, tests, builds, package scripts, and long-running processes. Call open_workspace first and pass workspaceId.", + inputSchema: { + workspaceId: z.string().describe("Workspace identifier returned by open_workspace."), + cmd: z.string().min(1).describe("Shell command to execute."), + tty: z + .boolean() + .optional() + .describe("Allocate a pseudo-terminal for interactive commands. Defaults to false."), + columns: z.number().int().min(1).max(1_000).optional().describe("Initial PTY width. Defaults to 80."), + rows: z.number().int().min(1).max(1_000).optional().describe("Initial PTY height. Defaults to 24."), + workingDirectory: z + .string() + .optional() + .describe("Working directory relative to the workspace root. Defaults to the workspace root."), + yieldTimeMs: z + .number() + .int() + .min(0) + .max(30_000) + .optional() + .describe("Milliseconds to wait before returning a running session. Defaults to 10000."), + maxOutputTokens: z + .number() + .int() + .positive() + .max(100_000) + .optional() + .describe("Approximate output token budget. Defaults to 10000."), + }, + outputSchema: processOutputSchema(), + ...toolWidgetDescriptorMeta(config, "shell"), + annotations: SHELL_TOOL_ANNOTATIONS, + }, + async ({ workspaceId, cmd, tty, columns, rows, workingDirectory, yieldTimeMs, maxOutputTokens }) => { + const startedAt = performance.now(); + const workspace = workspaces.getWorkspace(workspaceId); + const cwd = workspaces.resolveWorkingDirectory(workspace, workingDirectory); + const snapshot = await processSessions.start({ + workspaceId, + command: cmd, + cwd, + tty, + columns, + rows, + yieldTimeMs, + maxOutputTokens, + }); + + logToolCall(config, { + tool: "exec_command", + workspaceId, + workingDirectory: workingDirectory ?? ".", + command: cmd, + commandLength: cmd.length, + success: true, + durationMs: Math.round(performance.now() - startedAt), + }); + + return processToolResponse("exec_command", workspaceId, snapshot, { + command: cmd, + workingDirectory: workingDirectory ?? ".", + running: snapshot.running, + exitCode: snapshot.exitCode, + wallTimeMs: snapshot.wallTimeMs, + }); + }, + ); + + registerAppTool( + server, + "write_stdin", + { + title: "Write to process", + description: + "Poll or write characters to a process returned by exec_command. Omit chars or pass an empty string to poll. Pass \\u0003 to send Ctrl-C.", + inputSchema: { + workspaceId: z.string().describe("Workspace identifier used to start the process."), + sessionId: z.number().describe("Process session identifier returned by exec_command."), + chars: z.string().optional().describe("Characters to write. Omit or pass an empty string to poll."), + columns: z.number().int().min(1).max(1_000).optional().describe("Resize a PTY to this width."), + rows: z.number().int().min(1).max(1_000).optional().describe("Resize a PTY to this height."), + yieldTimeMs: z + .number() + .int() + .min(0) + .max(30_000) + .optional() + .describe("Milliseconds to wait for process output or completion. Defaults to 10000."), + maxOutputTokens: z + .number() + .int() + .positive() + .max(100_000) + .optional() + .describe("Approximate output token budget. Defaults to 10000."), + }, + outputSchema: processOutputSchema(), + ...toolWidgetDescriptorMeta(config, "shell"), + annotations: SHELL_TOOL_ANNOTATIONS, + }, + async ({ workspaceId, sessionId, chars, columns, rows, yieldTimeMs, maxOutputTokens }) => { + const startedAt = performance.now(); + workspaces.getWorkspace(workspaceId); + const snapshot = await processSessions.write({ + workspaceId, + sessionId, + chars, + columns, + rows, + yieldTimeMs, + maxOutputTokens, + }); + + logToolCall(config, { + tool: "write_stdin", + workspaceId, + success: true, + durationMs: Math.round(performance.now() - startedAt), + }); + + return processToolResponse("write_stdin", workspaceId, snapshot, { + sessionId, + charactersWritten: chars?.length ?? 0, + running: snapshot.running, + exitCode: snapshot.exitCode, + wallTimeMs: snapshot.wallTimeMs, + }); + }, + ); +} + function createMcpServer( config: ServerConfig, workspaces: WorkspaceRegistry, reviewCheckpoints: ReturnType, + processSessions: ProcessSessionManager, ): McpServer { - const toolNames = toolNamesFor(config); const server = new McpServer( { name: "devspace", @@ -473,7 +649,7 @@ function createMcpServer( "Secure local coding workspace for MCP clients. Provides workspace-scoped file, search, edit, write, and shell tools.", }, { - instructions: serverInstructions(config, toolNames), + instructions: serverInstructions(config), }, ); @@ -739,6 +915,7 @@ function createMcpServer( }, ); + if (config.toolMode !== "codex") { registerAppTool( server, toolNames.write, @@ -902,6 +1079,81 @@ function createMcpServer( }; }, ); + } + + if (config.toolMode === "codex") { + registerAppTool( + server, + "apply_patch", + { + title: "Apply patch", + description: + "Apply one Codex-style patch inside an open workspace. Supports adding, overwriting, updating, deleting, and moving files. Use this for all file modifications. Paths must be relative to the workspace. Call open_workspace first and pass workspaceId.", + inputSchema: { + workspaceId: z + .string() + .describe("Workspace identifier returned by open_workspace."), + patch: z + .string() + .describe("Patch text enclosed by *** Begin Patch and *** End Patch markers."), + }, + outputSchema: resultOutputSchema({ + additions: z.number(), + removals: z.number(), + files: z.array( + z.object({ + path: z.string(), + previousPath: z.string().optional(), + operation: z.enum(["add", "update", "delete", "move"]), + }), + ), + }), + ...toolWidgetDescriptorMeta(config, "edit"), + annotations: EDIT_TOOL_ANNOTATIONS, + }, + async ({ workspaceId, patch }) => { + const startedAt = performance.now(); + const workspace = workspaces.getWorkspace(workspaceId); + const applied = await applyPatch(workspace.root, patch); + const paths = applied.files.map((file) => file.path).join(", "); + const result = `Applied patch to ${applied.files.length} file(s): ${paths}`; + const content = [textBlock(result)]; + const displayPath = applied.files.length === 1 + ? applied.files[0]?.path + : `${applied.files.length} files`; + + logToolCall(config, { + tool: "apply_patch", + workspaceId, + success: true, + durationMs: Math.round(performance.now() - startedAt), + }); + + return { + content, + _meta: { + tool: "apply_patch", + card: { + workspaceId, + path: displayPath, + summary: { + files: applied.files.length, + additions: applied.additions, + removals: applied.removals, + }, + payload: { patch: applied.patch }, + }, + }, + structuredContent: { + result, + additions: applied.additions, + removals: applied.removals, + files: applied.files, + }, + }; + }, + ); + } if (config.widgets === "changes") { registerAppTool( @@ -910,32 +1162,24 @@ function createMcpServer( { title: "Show changes", description: - "Show aggregate file changes in an open workspace since the last shown checkpoint or since the workspace was opened. After you create, edit, or overwrite files, call this once when the related file changes are complete so the user can inspect the combined diff.", + "Show aggregate file changes for an open workspace. If the current turn successfully modified files, call this exactly once after the final related file change and before your final response so the user can inspect the combined diff for the turn. Do not call it after every individual file change, and do not skip it because prior file-change tools already displayed per-tool diffs.", inputSchema: { workspaceId: z .string() .describe("Workspace identifier returned by open_workspace."), - since: z - .enum(["last_shown", "workspace_open"]) - .optional() - .describe("Defaults to last_shown. Use workspace_open to compare against the initial open_workspace checkpoint."), - markReviewed: z - .boolean() - .optional() - .describe("Defaults to true. When true, advances the last shown checkpoint to the current workspace state."), }, outputSchema: resultOutputSchema(), ...toolWidgetDescriptorMeta(config, "show_changes"), annotations: { readOnlyHint: true }, }, - async ({ workspaceId, since, markReviewed }) => { + async ({ workspaceId }) => { const startedAt = performance.now(); const workspace = workspaces.getWorkspace(workspaceId); const review = await reviewCheckpoints.reviewChanges({ workspaceId, root: workspace.root, - since: since ?? "last_shown", - markReviewed: markReviewed ?? true, + since: "last_shown", + markReviewed: true, }); const content = [textBlock(review.result)]; @@ -967,12 +1211,12 @@ function createMcpServer( ); } - if (!config.minimalTools) { + if (config.toolMode === "full") { registerAppTool( server, toolNames.grep, { - title: config.toolNaming === "short" ? "Grep" : "Grep files", + title: "Grep", description: "Search file contents inside an open workspace. Use this before broad reads when looking for symbols, text, or usage sites. Respects project ignore rules. Call open_workspace first and pass workspaceId.", inputSchema: { @@ -1045,7 +1289,7 @@ function createMcpServer( server, toolNames.glob, { - title: config.toolNaming === "short" ? "Glob" : "Find files", + title: "Glob", description: "Find files by glob pattern inside an open workspace. Use this to discover filenames or narrow file sets before reading. Respects project ignore rules. Call open_workspace first and pass workspaceId.", inputSchema: { @@ -1115,7 +1359,7 @@ function createMcpServer( server, toolNames.ls, { - title: config.toolNaming === "short" ? "Ls" : "List directory", + title: "Ls", description: "List a directory inside an open workspace. Use this for directory inspection before reading files. Call open_workspace first and pass workspaceId.", inputSchema: { @@ -1178,12 +1422,13 @@ function createMcpServer( ); } + if (config.toolMode !== "codex") { registerAppTool( server, toolNames.shell, { - title: config.toolNaming === "short" ? "Bash" : "Run shell", - description: config.minimalTools + title: "Bash", + description: config.toolMode !== "full" ? `Run a shell command inside an open workspace. Use only for tests, builds, git inspection, package scripts, search, file discovery, and directory inspection. In minimal tool mode, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} are disabled; use command-line tools such as grep, rg, find, ls, and tree for those read-only inspection actions. Do not use ${toolNames.shell} to create or modify files. Do not use shell redirection, heredocs, tee, sed -i, perl -i, node/python/ruby scripts, or generated scripts to write project files; use ${toolNames.edit} for targeted changes and ${toolNames.write} for new files or full rewrites. Prefer ${toolNames.read} for direct file reads. Call open_workspace first and pass workspaceId. This is powerful local execution and should only be exposed behind strong authentication.` : `Run a shell command inside an open workspace. Use only for tests, builds, git inspection, package scripts, and commands that are better executed by the shell. Do not use ${toolNames.shell} to create or modify files. Do not use shell redirection, heredocs, tee, sed -i, perl -i, node/python/ruby scripts, or generated scripts to write project files; use ${toolNames.edit} for targeted changes and ${toolNames.write} for new files or full rewrites. Prefer ${toolNames.read}, ${toolNames.grep}, ${toolNames.glob}, and ${toolNames.ls} for file inspection. Call open_workspace first and pass workspaceId. This is powerful local execution and should only be exposed behind strong authentication.`, inputSchema: { @@ -1267,6 +1512,11 @@ function createMcpServer( }; }, ); + } + + if (config.toolMode === "codex") { + registerCodexProcessTools(server, config, workspaces, processSessions); + } return server; } @@ -1291,6 +1541,7 @@ export function createServer(config = loadConfig()): RunningServer { const workspaceStore = createWorkspaceStore(config.stateDir); const workspaces = new WorkspaceRegistry(config, workspaceStore); const reviewCheckpoints = createReviewCheckpointManager(); + const processSessions = new ProcessSessionManager(); // Hops gate is authoritative when set; otherwise preserve upstream boolean behavior. if (config.trustProxyHops !== undefined) { @@ -1417,7 +1668,7 @@ export function createServer(config = loadConfig()): RunningServer { } }; - const server = createMcpServer(config, workspaces, reviewCheckpoints); + const server = createMcpServer(config, workspaces, reviewCheckpoints, processSessions); await server.connect(transport); } else { sendJsonRpcError(res, 400, -32000, "No valid MCP session"); @@ -1443,6 +1694,7 @@ export function createServer(config = loadConfig()): RunningServer { close: () => { if (closed) return; closed = true; + processSessions.shutdown(); oauthProvider.close(); workspaceStore.close?.(); }, diff --git a/src/skills.test.ts b/src/skills.test.ts index 1b0ebae7..16a49c8a 100644 --- a/src/skills.test.ts +++ b/src/skills.test.ts @@ -4,22 +4,79 @@ import { join } from "node:path"; import assert from "node:assert/strict"; import { loadConfig } from "./config.js"; import { + effectiveSkillPaths, formatPathForPrompt, loadWorkspaceSkills, resolveSkillReadPath, } from "./skills.js"; const root = await mkdtemp(join(tmpdir(), "devspace-skills-test-")); +const originalHome = process.env.HOME; +const originalUserProfile = process.env.USERPROFILE; try { + process.env.HOME = root; + process.env.USERPROFILE = root; const projectRoot = join(root, "project"); const agentDir = join(root, "agent"); const explicitSkills = join(root, "explicit-skills"); + const globalAgentsSkills = join(root, ".agents", "skills"); + const projectAgentsSkills = join(projectRoot, ".agents", "skills"); + const globalClaudeSkills = join(root, ".claude", "skills"); + const projectClaudeSkills = join(projectRoot, ".claude", "skills"); + await mkdir(join(globalAgentsSkills, "agent-global-skill"), { recursive: true }); + await mkdir(join(projectAgentsSkills, "agent-project-skill"), { recursive: true }); + await mkdir(join(globalClaudeSkills, "claude-global-skill"), { recursive: true }); + await mkdir(join(projectClaudeSkills, "claude-project-skill"), { recursive: true }); await mkdir(join(projectRoot, ".pi", "skills", "project-skill"), { recursive: true }); await mkdir(join(agentDir, "skills", "global-skill"), { recursive: true }); await mkdir(join(explicitSkills, "duplicate"), { recursive: true }); await mkdir(join(explicitSkills, "disabled"), { recursive: true }); + await writeFile( + join(globalAgentsSkills, "agent-global-skill", "SKILL.md"), + [ + "---", + "name: agent-global-skill", + "description: Agent global skill description.", + "---", + "", + "# Agent Global Skill", + ].join("\n"), + ); + await writeFile( + join(projectAgentsSkills, "agent-project-skill", "SKILL.md"), + [ + "---", + "name: agent-project-skill", + "description: Agent project skill description.", + "---", + "", + "# Agent Project Skill", + ].join("\n"), + ); + await writeFile( + join(globalClaudeSkills, "claude-global-skill", "SKILL.md"), + [ + "---", + "name: claude-global-skill", + "description: Claude global skill description.", + "---", + "", + "# Claude Global Skill", + ].join("\n"), + ); + await writeFile( + join(projectClaudeSkills, "claude-project-skill", "SKILL.md"), + [ + "---", + "name: claude-project-skill", + "description: Claude project skill description.", + "---", + "", + "# Claude Project Skill", + ].join("\n"), + ); await writeFile( join(projectRoot, ".pi", "skills", "project-skill", "SKILL.md"), [ @@ -79,17 +136,45 @@ try { const config = loadConfig({ DEVSPACE_ALLOWED_ROOTS: projectRoot, DEVSPACE_AGENT_DIR: agentDir, - DEVSPACE_SKILL_PATHS: explicitSkills, + DEVSPACE_SKILL_PATHS: [explicitSkills, "~/.claude/skills", "./.claude/skills"].join(","), DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", PORT: "1", }); const loaded = loadWorkspaceSkills(config, projectRoot); - assert.equal(loaded.skills.some((skill) => skill.name === "project-skill"), true); + assert.equal(loaded.skills.some((skill) => skill.name === "agent-global-skill"), true); + assert.equal(loaded.skills.some((skill) => skill.name === "agent-project-skill"), true); + assert.equal(loaded.skills.some((skill) => skill.name === "claude-global-skill"), true); + assert.equal(loaded.skills.some((skill) => skill.name === "claude-project-skill"), true); + assert.equal(loaded.skills.some((skill) => skill.name === "project-skill"), 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.diagnostics.some((diagnostic) => diagnostic.type === "collision"), true); - const projectSkill = loaded.skills.find((skill) => skill.name === "project-skill"); + const duplicateConfig = loadConfig({ + DEVSPACE_ALLOWED_ROOTS: projectRoot, + DEVSPACE_AGENT_DIR: agentDir, + DEVSPACE_SKILL_PATHS: [explicitSkills, "./.agents/skills"].join(","), + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + assert.equal( + effectiveSkillPaths(duplicateConfig, projectRoot).filter((path) => path === projectAgentsSkills).length, + 1, + ); + + const legacyPiConfig = loadConfig({ + DEVSPACE_ALLOWED_ROOTS: projectRoot, + DEVSPACE_AGENT_DIR: agentDir, + DEVSPACE_SKILL_PATHS: [explicitSkills, join(projectRoot, ".pi", "skills")].join(","), + DEVSPACE_OAUTH_OWNER_TOKEN: "test-owner-token-that-is-long-enough", + PORT: "1", + }); + assert.equal( + loadWorkspaceSkills(legacyPiConfig, projectRoot).skills.some((skill) => skill.name === "project-skill"), + true, + ); + + const projectSkill = loaded.skills.find((skill) => skill.name === "agent-project-skill"); assert.ok(projectSkill); assert.match(formatPathForPrompt(projectSkill.filePath), /SKILL\.md$/); @@ -106,5 +191,9 @@ try { false, ); } finally { + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; await rm(root, { recursive: true, force: true }); } diff --git a/src/skills.ts b/src/skills.ts index 20a35205..e3da62ff 100644 --- a/src/skills.ts +++ b/src/skills.ts @@ -1,5 +1,6 @@ +import { existsSync } from "node:fs"; import { homedir } from "node:os"; -import { resolve, sep } from "node:path"; +import { join, resolve, sep } from "node:path"; import { loadSkills, type Skill, @@ -19,14 +20,35 @@ export interface SkillReadResolution { isSkillFile: boolean; } +export function effectiveSkillPaths(config: ServerConfig, cwd: string): string[] { + const defaultPaths = [ + join(homedir(), ".agents", "skills"), + resolve(cwd, ".agents", "skills"), + join(config.agentDir, "skills"), + ].filter((path) => existsSync(path)); + + const seen = new Set(); + return [...defaultPaths, ...config.skillPaths] + .map((path) => resolveSkillPath(path, cwd)) + .filter((path) => { + if (seen.has(path)) return false; + seen.add(path); + return true; + }); +} + +function resolveSkillPath(path: string, cwd: string): string { + return resolve(cwd, expandHomePath(path)); +} + export function loadWorkspaceSkills(config: ServerConfig, cwd: string): LoadedSkills { if (!config.skillsEnabled) return { skills: [], diagnostics: [] }; return loadSkills({ cwd, agentDir: config.agentDir, - skillPaths: config.skillPaths, - includeDefaults: true, + skillPaths: effectiveSkillPaths(config, cwd), + includeDefaults: false, }); } diff --git a/src/ui/card-types.test.ts b/src/ui/card-types.test.ts new file mode 100644 index 00000000..eb47e9a0 --- /dev/null +++ b/src/ui/card-types.test.ts @@ -0,0 +1,25 @@ +import assert from "node:assert/strict"; +import { + isEditTool, + isExpandableCard, + isPatchTool, + isShellTool, + isToolName, +} from "./card-types.js"; + +for (const tool of ["apply_patch", "exec_command", "write_stdin"]) { + assert.equal(isToolName(tool), true, `${tool} should be a recognized card tool`); +} + +assert.equal(isPatchTool("apply_patch"), true); +assert.equal(isEditTool("apply_patch"), false); +assert.equal(isShellTool("exec_command"), true); +assert.equal(isShellTool("write_stdin"), true); +assert.equal(isEditTool("exec_command"), false); +assert.equal(isShellTool("apply_patch"), false); + +assert.equal( + isExpandableCard({ tool: "apply_patch", payload: { patch: "diff --git a/a b/a" } }), + true, +); +assert.equal(isExpandableCard({ tool: "apply_patch" }), false); diff --git a/src/ui/card-types.ts b/src/ui/card-types.ts index 89ec3fe6..1e3c9409 100644 --- a/src/ui/card-types.ts +++ b/src/ui/card-types.ts @@ -2,14 +2,10 @@ import type { App } from "@modelcontextprotocol/ext-apps"; export type ToolName = | "open_workspace" - | "read_file" - | "write_file" - | "edit_file" - | "grep_files" - | "find_files" - | "list_directory" - | "run_shell" | "show_changes" + | "apply_patch" + | "exec_command" + | "write_stdin" | "read" | "write" | "edit" @@ -20,6 +16,8 @@ export type ToolName = export type HostContext = NonNullable>; +export type PatchOperation = "add" | "update" | "delete" | "move"; + export interface ToolResultCard { tool: ToolName; workspaceId?: string; @@ -30,6 +28,7 @@ export interface ToolResultCard { files?: Array<{ path?: string; previousPath?: string; + operation?: PatchOperation; type?: string; additions?: number; removals?: number; @@ -67,14 +66,10 @@ export interface ToolPayload { export function isToolName(value: unknown): value is ToolName { return ( value === "open_workspace" || - value === "read_file" || - value === "write_file" || - value === "edit_file" || - value === "grep_files" || - value === "find_files" || - value === "list_directory" || - value === "run_shell" || value === "show_changes" || + value === "apply_patch" || + value === "exec_command" || + value === "write_stdin" || value === "read" || value === "write" || value === "edit" || @@ -86,23 +81,27 @@ export function isToolName(value: unknown): value is ToolName { } export function isReadTool(tool: ToolName): boolean { - return tool === "read_file" || tool === "read"; + return tool === "read"; } export function isWriteTool(tool: ToolName): boolean { - return tool === "write_file" || tool === "write"; + return tool === "write"; } export function isEditTool(tool: ToolName): boolean { - return tool === "edit_file" || tool === "edit"; + return tool === "edit"; +} + +export function isPatchTool(tool: ToolName): boolean { + return tool === "apply_patch"; } export function isSearchTool(tool: ToolName): boolean { - return tool === "grep_files" || tool === "find_files" || tool === "grep" || tool === "glob"; + return tool === "grep" || tool === "glob"; } export function isShellTool(tool: ToolName): boolean { - return tool === "run_shell" || tool === "bash"; + return tool === "bash" || tool === "exec_command" || tool === "write_stdin"; } export function isReviewTool(tool: ToolName): boolean { @@ -147,6 +146,7 @@ export function isExpandableCard(card: ToolResultCard): boolean { } if (isReviewTool(card.tool)) return Boolean(card.files?.length || card.payload?.patch); + if (isPatchTool(card.tool)) return Boolean(card.payload?.patch); return Boolean(card.payload); } diff --git a/src/ui/patch-display.test.ts b/src/ui/patch-display.test.ts new file mode 100644 index 00000000..5583acea --- /dev/null +++ b/src/ui/patch-display.test.ts @@ -0,0 +1,70 @@ +import assert from "node:assert/strict"; +import { getPatchDisplayParts } from "./patch-display.js"; + +assert.deepEqual(getPatchDisplayParts({}), { + title: "Apply Patch", + tone: "edit", +}); + +assert.deepEqual( + getPatchDisplayParts({ files: [{ path: "created.ts", operation: "add" }] }), + { + title: "Write File", + iconOperation: "add", + tone: "write", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "a.ts", operation: "add" }, + { path: "b.ts", operation: "add" }, + ], + }), + { + title: "Write Files", + iconOperation: "add", + tone: "write", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "created.ts", operation: "add" }, + { path: "edited.ts", operation: "update" }, + ], + }), + { + title: "Write & Edit Files", + tone: "edit", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "same.ts", operation: "add" }, + { path: "same.ts", operation: "update" }, + ], + }), + { + title: "Write & Edit File", + tone: "edit", + }, +); + +assert.deepEqual( + getPatchDisplayParts({ + files: [ + { path: "edited.ts", operation: "update" }, + { path: "moved.ts", previousPath: "old.ts", operation: "move" }, + { path: "removed.ts", operation: "delete" }, + ], + }), + { + title: "Edit, Move & Delete Files", + tone: "edit", + }, +); diff --git a/src/ui/patch-display.ts b/src/ui/patch-display.ts new file mode 100644 index 00000000..92632331 --- /dev/null +++ b/src/ui/patch-display.ts @@ -0,0 +1,74 @@ +import type { PatchOperation, ToolResultCard } from "./card-types.js"; + +export interface PatchDisplayParts { + title: string; + iconOperation?: PatchOperation; + tone: "edit" | "write"; +} + +const patchOperationLabels: Record = { + add: "Write", + update: "Edit", + delete: "Delete", + move: "Move", +}; + +export function getPatchDisplayParts(card: Pick): PatchDisplayParts { + const files = card.files ?? []; + const operations = patchOperations(files); + + if (operations.length === 0) { + return { title: "Apply Patch", tone: "edit" }; + } + + const singleOperation = operations.length === 1 ? operations[0] : undefined; + + const display: PatchDisplayParts = { + title: patchTitle(operations, countChangedFiles(files)), + tone: singleOperation === "add" ? "write" : "edit", + }; + if (singleOperation) display.iconOperation = singleOperation; + return display; +} + +function patchOperations(files: NonNullable): PatchOperation[] { + const operations = new Set(); + for (const file of files) { + if (file.operation) operations.add(file.operation); + } + return [...operations]; +} + +function countChangedFiles(files: NonNullable): number { + const paths = new Set(); + let unnamedFiles = 0; + + for (const file of files) { + const path = file.path ?? file.previousPath; + if (path) { + paths.add(path); + } else { + unnamedFiles += 1; + } + } + + return paths.size + unnamedFiles; +} + +function patchTitle(operations: PatchOperation[], fileCount: number): string { + if (operations.length === 1) { + return `${patchOperationLabels[operations[0]]} ${fileNoun(fileCount)}`; + } + + return `${joinTitleParts(operations.map((operation) => patchOperationLabels[operation]))} ${fileNoun(fileCount)}`; +} + +function fileNoun(fileCount: number): "File" | "Files" { + return fileCount === 1 ? "File" : "Files"; +} + +function joinTitleParts(parts: string[]): string { + if (parts.length <= 1) return parts[0] ?? ""; + if (parts.length === 2) return `${parts[0]} & ${parts[1]}`; + return `${parts.slice(0, -1).join(", ")} & ${parts.at(-1)}`; +} diff --git a/src/ui/workspace-app.css b/src/ui/workspace-app.css index 0f5003f4..28ba6bf5 100644 --- a/src/ui/workspace-app.css +++ b/src/ui/workspace-app.css @@ -366,7 +366,6 @@ body { word-break: break-word; } -.text-payload.run_shell, .text-payload.bash { color: var(--color-text-primary, #f5f5f6); background: var(--color-background-primary, #101114); diff --git a/src/ui/workspace-app.tsx b/src/ui/workspace-app.tsx index 373b3ac2..edbb620f 100644 --- a/src/ui/workspace-app.tsx +++ b/src/ui/workspace-app.tsx @@ -8,6 +8,7 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { isEditTool, isExpandableCard, + isPatchTool, isReadTool, isReviewTool, isSearchTool, @@ -18,9 +19,11 @@ import { payloadText, summaryNumber, type HostContext, + type PatchOperation, type ToolName, type ToolResultCard, } from "./card-types.js"; +import { getPatchDisplayParts } from "./patch-display.js"; import "./workspace-app.css"; interface ToolDisplay { @@ -261,17 +264,17 @@ async function renderPayloadIfNeeded(): Promise { return; } - if (isReviewTool(card.tool)) { - const visibleFileCount = reviewFilesExpanded - ? undefined - : Math.max(3, (card.files ?? []).slice(0, 3).length); + if (isReviewTool(card.tool) || isPatchTool(card.tool)) { + const visibleFileCount = isReviewTool(card.tool) && !reviewFilesExpanded + ? Math.max(3, (card.files ?? []).slice(0, 3).length) + : undefined; if (currentPayload) { currentPayload.update({ card, hostContext, errorMessage, visibleFileCount }); return; } - renderStatus(target, "Loading review..."); + renderStatus(target, isReviewTool(card.tool) ? "Loading review..." : "Loading diff..."); const { mountReviewPayload } = await import("./review-payload.js"); if (target !== currentPayloadContainer || !card) return; @@ -340,7 +343,7 @@ function renderSummaryBadge(card: ToolResultCard): HTMLElement { return stats; } - if (isEditTool(card.tool) || isWriteTool(card.tool)) { + if (isPatchTool(card.tool) || isEditTool(card.tool) || isWriteTool(card.tool)) { const stats = element("span", { className: "stats" }); stats.setAttribute("aria-label", "Diff statistics"); stats.append( @@ -369,7 +372,11 @@ function renderSummaryBadge(card: ToolResultCard): HTMLElement { } if (isShellTool(card.tool)) { - return element("span", { className: "badge", text: `ran ยท ${String(summary.lines ?? 0)} lines` }); + const state = summary.running === true ? "running" : "ran"; + return element("span", { + className: "badge", + text: `${state} ยท ${String(summary.lines ?? 0)} lines`, + }); } if (isSearchTool(card.tool)) { @@ -486,33 +493,50 @@ function formatAgentsFilesForPayload( .join("\n\n"); } +function getPatchToolDisplay(card: ToolResultCard, label: string): ToolDisplay { + const display = getPatchDisplayParts(card); + + return { + icon: patchIcon(display.iconOperation), + title: display.title, + label, + tone: display.tone, + }; +} + +function patchIcon(operation: PatchOperation | undefined): string { + if (operation === "add") return filePlusIcon(); + if (operation === "delete") return fileIcon(); + if (operation === "move") return filesIcon(); + return editIcon(); +} + function getToolDisplay(card: ToolResultCard): ToolDisplay { const label = getToolLabel(card); switch (card.tool) { case "open_workspace": return { icon: folderIcon(), title: "Workspace", label, tone: "workspace" }; - case "read_file": case "read": return { icon: fileIcon(), title: "Read File", label, tone: "read" }; - case "write_file": case "write": return { icon: filePlusIcon(), title: "Write File", label, tone: "write" }; - case "edit_file": case "edit": return { icon: editIcon(), title: "Edit File", label, tone: "edit" }; - case "grep_files": + case "apply_patch": + return getPatchToolDisplay(card, label); case "grep": return { icon: searchIcon(), title: "Grep", label, tone: "search" }; - case "find_files": case "glob": return { icon: filesIcon(), title: "Glob", label, tone: "search" }; - case "list_directory": case "ls": return { icon: listIcon(), title: "List Directory", label, tone: "directory" }; - case "run_shell": case "bash": return { icon: terminalIcon(), title: "Bash", label, tone: "shell" }; + case "exec_command": + return { icon: terminalIcon(), title: "Exec Command", label, tone: "shell" }; + case "write_stdin": + return { icon: terminalIcon(), title: "Process Session", label, tone: "shell" }; case "show_changes": return { icon: reviewIcon(), title: "Show Changes", label, tone: "review" }; } @@ -520,7 +544,7 @@ function getToolDisplay(card: ToolResultCard): ToolDisplay { function getToolLabel(card: ToolResultCard): string { if (isShellTool(card.tool)) { - return String(card.summary?.command ?? card.path ?? card.tool); + return String(card.summary?.command ?? card.summary?.sessionId ?? card.path ?? card.tool); } if (isReviewTool(card.tool)) { const count = Number(card.summary?.files ?? card.files?.length ?? 0);