diff --git a/TEST_IMPROVEMENTS.md b/TEST_IMPROVEMENTS.md new file mode 100644 index 00000000000..861a9121c2f --- /dev/null +++ b/TEST_IMPROVEMENTS.md @@ -0,0 +1,90 @@ +# Test Improvements for MCP Headless Mode (PR #9328) + +## Summary + +This PR adds comprehensive test coverage for the `allowHeadless` feature introduced in PR #9328. The new tests ensure robust behavior of MCP tools in headless mode across various scenarios. + +## Test Coverage Added + +### 1. **Edge Cases and Error Handling** (`MCP tool edge cases and error handling`) + +- **Disconnected servers**: Verifies graceful handling when MCP server is disconnected +- **Empty connections**: Tests behavior when no MCP connections are configured +- **Empty tools array**: Ensures no errors when a server has no tools +- **Explicit false**: Confirms tools are excluded when `allowHeadless: false` is explicit + +### 2. **Permission Policy Interactions** (`MCP tool permission policy interactions`) + +- **Wildcard exclude over allowHeadless**: Ensures wildcard excludes take precedence +- **Specific allow vs wildcard ask**: Tests policy priority resolution +- **allowHeadless with wildcard ask**: Verifies upgrade behavior in headless mode +- **Wildcard ask without allowHeadless**: Confirms denial in headless mode +- **argumentMatches with allowHeadless**: Tests argument-based permission matching + +### 3. **Integration Tests** (`MCP tool integration tests`) + +- **Mixed built-in and MCP tools**: Verifies correct filtering in headless mode +- **Tool order preservation**: Ensures filtering maintains tool ordering +- **Interactive to headless transition**: Tests mode switching behavior + +### 4. **Built-in Tool Behavior** + +- **Built-in tools in headless**: Confirms built-in tools work regardless of `allowHeadless` + +## Test Scenarios + +### Security-Critical Tests + +1. ✅ Explicit exclusions always respected (cannot be bypassed by `allowHeadless`) +2. ✅ Wildcard excludes take precedence over `allowHeadless` +3. ✅ Default behavior is secure (tools excluded without explicit `allowHeadless: true`) + +### Functionality Tests + +1. ✅ Tools with `allowHeadless: true` work in headless mode +2. ✅ Tools with `allowHeadless: false` are excluded in headless mode +3. ✅ Tools with `allowHeadless: undefined` are excluded in headless mode +4. ✅ All MCP tools work in interactive mode regardless of `allowHeadless` + +### Integration Tests + +1. ✅ Multiple servers with different `allowHeadless` settings +2. ✅ Built-in tools always available in headless mode +3. ✅ Tool enumeration maintains consistent ordering +4. ✅ Mode transitions handled correctly + +## Key Improvements Over Original Tests + +1. **Comprehensive edge case coverage**: Tests error conditions and boundary cases +2. **Permission policy interactions**: Validates complex policy scenarios with wildcards +3. **Integration testing**: Ensures components work together correctly +4. **Real-world scenarios**: Tests common use cases like mixed servers +5. **Security validation**: Explicitly tests that security cannot be bypassed + +## Test Organization + +Tests are organized into 4 logical groups: + +- `MCP tools in headless mode` (original tests from PR #9328) +- `MCP tool execution permission in headless mode` (original + new built-in test) +- `MCP tool edge cases and error handling` (new) +- `MCP tool permission policy interactions` (new) +- `MCP tool integration tests` (new) + +## Files Modified + +- `extensions/cli/src/stream/mcp-headless.test.ts`: Added 12 new test cases + +## Test Execution + +Run tests with: + +```bash +cd extensions/cli +npm test -- mcp-headless.test.ts +``` + +## Related PRs + +- PR #9328: Original feature implementation and documentation +- PR #9327: Parent PR for allowHeadless feature diff --git a/extensions/cli/package-lock.json b/extensions/cli/package-lock.json index b86896f4f0a..34d2ce35145 100644 --- a/extensions/cli/package-lock.json +++ b/extensions/cli/package-lock.json @@ -120,9 +120,9 @@ "license": "Apache-2.0", "dependencies": { "@anthropic-ai/sdk": "^0.62.0", - "@aws-sdk/client-bedrock-runtime": "^3.779.0", + "@aws-sdk/client-bedrock-runtime": "^3.931.0", "@aws-sdk/client-sagemaker-runtime": "^3.777.0", - "@aws-sdk/credential-providers": "^3.778.0", + "@aws-sdk/credential-providers": "^3.931.0", "@continuedev/config-types": "^1.0.13", "@continuedev/config-yaml": "file:../packages/config-yaml", "@continuedev/fetch": "file:../packages/fetch", @@ -275,8 +275,8 @@ "@ai-sdk/anthropic": "^1.0.10", "@ai-sdk/openai": "^1.0.10", "@anthropic-ai/sdk": "^0.67.0", - "@aws-sdk/client-bedrock-runtime": "^3.929.0", - "@aws-sdk/credential-providers": "^3.929.0", + "@aws-sdk/client-bedrock-runtime": "^3.931.0", + "@aws-sdk/credential-providers": "^3.931.0", "@continuedev/config-types": "^1.0.14", "@continuedev/config-yaml": "^1.36.0", "@continuedev/fetch": "^1.6.0", @@ -520,7 +520,6 @@ } ], "license": "MIT", - "peer": true, "engines": { "node": ">=18" }, @@ -544,7 +543,6 @@ } ], "license": "MIT", - "peer": true, "engines": { "node": ">=18" } @@ -1809,7 +1807,6 @@ "integrity": "sha512-oNXsh2ywth5aowwIa7RKtawnkdH6LgU1ztfP9AIUCQCvzysB+WeU8o2kyyosDPwBZutPpjZDKPQGIzzrfTWweQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.1", @@ -1980,7 +1977,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=8.0.0" } @@ -2002,7 +1998,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/context-async-hooks/-/context-async-hooks-2.0.1.tgz", "integrity": "sha512-XuY23lSI3d4PEqKA+7SLtAgwqIfc6E/E9eAQWLN1vlpC53ybO3o6jW4BsXo1xvz9lYyyWItfQDDLzezER01mCw==", "license": "Apache-2.0", - "peer": true, "engines": { "node": "^18.19.0 || >=20.6.0" }, @@ -2015,7 +2010,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.0.1.tgz", "integrity": "sha512-MaZk9SJIDgo1peKevlbhP6+IwIiNPNmswNL4AF0WaQJLbHXjr9SrZMgS12+iqr9ToV4ZVosCcc0f8Rg67LXjxw==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/semantic-conventions": "^1.29.0" }, @@ -2257,7 +2251,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/instrumentation/-/instrumentation-0.203.0.tgz", "integrity": "sha512-ke1qyM+3AK2zPuBPb6Hk/GCsc5ewbLvPNkEuELx/JmANeEp6ZjnZ+wypPAJSucTw0wvCGrUaibDSdcrGFoWxKQ==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/api-logs": "0.203.0", "import-in-the-middle": "^1.8.1", @@ -3574,7 +3567,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/resources/-/resources-2.0.1.tgz", "integrity": "sha512-dZOB3R6zvBwDKnHDTB4X1xtMArB/d324VsbiPkX/Yu0Q8T2xceRthoIVFhJdvgVM2QhGVUyX9tzwiNxGtoBJUw==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/semantic-conventions": "^1.29.0" @@ -3663,7 +3655,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/sdk-trace-base/-/sdk-trace-base-2.0.1.tgz", "integrity": "sha512-xYLlvk/xdScGx1aEqvxLwf6sXQLXCjk3/1SQT9X9AoN5rXRhkdvIFShuNNmtTEPRBqcsMbS4p/gJLNI2wXaDuQ==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@opentelemetry/core": "2.0.1", "@opentelemetry/resources": "2.0.1", @@ -3699,7 +3690,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.36.0.tgz", "integrity": "sha512-TtxJSRD8Ohxp6bKkhrm27JRHAxPczQA7idtcTOMYI+wQRRrfgqxHv1cFbCApcSnNjtXkmzFozn6jQtFrOmbjPQ==", "license": "Apache-2.0", - "peer": true, "engines": { "node": ">=14" } @@ -5110,7 +5100,8 @@ "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/@types/body-parser": { "version": "1.19.6", @@ -5398,7 +5389,6 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-24.3.0.tgz", "integrity": "sha512-aPTXCrfwnDLj4VvXrm+UUCQjNEvJgNA8s5F1cvwQU+3KNltTOkBm1j30uNLyqqPNe7gE3KFzImYoZEfLhp4Yow==", "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.10.0" } @@ -5468,7 +5458,6 @@ "integrity": "sha512-lr3jdBw/BGj49Eps7EvqlUaoeA0xpj3pc0RoJkHpYaCHkVK7i28dKyImLQb3JVlqs3aYSXf7qYuWOW/fgZnTXQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "csstype": "^3.0.2" } @@ -5590,7 +5579,6 @@ "integrity": "sha512-w/EboPlBwnmOBtRbiOvzjD+wdiZdgFeo17lkltrtn7X37vagKKWJABvyfsJXTlHe6XBzugmYgd4A4nW+k8Mixw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/regexpp": "^4.10.0", "@typescript-eslint/scope-manager": "8.40.0", @@ -5621,7 +5609,6 @@ "integrity": "sha512-jCNyAuXx8dr5KJMkecGmZ8KI61KBUhkCob+SD+C+I5+Y1FWI2Y3QmY4/cxMCC5WAsZqoEtEETVhUiUMIGCf6Bw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.40.0", "@typescript-eslint/types": "8.40.0", @@ -6222,7 +6209,6 @@ "integrity": "sha512-hGISOaP18plkzbWEcP/QvtRW1xDXF2+96HbEX6byqQhAUbiS5oH6/9JwW+QsQCIYON2bI6QZBF+2PvOmrRZ9wA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "fflate": "^0.8.2", @@ -6303,7 +6289,6 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -6511,6 +6496,7 @@ "integrity": "sha512-b0P0sZPKtyu8HkeRAfCq0IfURZK+SuwMjY1UXGBU27wpAiTwQAIlq56IbIO+ytk/JjS1fMR14ee5WBBfKi5J6A==", "dev": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "dequal": "^2.0.3" } @@ -8384,7 +8370,8 @@ "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/dot-prop": { "version": "5.3.0", @@ -8925,7 +8912,6 @@ "integrity": "sha512-TS9bTNIryDzStCpJN93aC5VRSW3uTx9sClUn4B87pwiCaJh220otoI0X8mJKr+VcPtniMdN8GKjlwgWGUv5ZKA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.12.1", @@ -9097,7 +9083,6 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -9545,7 +9530,6 @@ "integrity": "sha512-hIS4idWWai69NezIdRt2xFVofaF4j+6INOpJlVOLDO8zXGpUVEVzIYk12UUi2JzjEzWL3IOAxcTubgz9Po0yXw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "accepts": "^2.0.0", "body-parser": "^2.2.1", @@ -12142,6 +12126,7 @@ "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", "dev": true, "license": "MIT", + "peer": true, "bin": { "lz-string": "bin/bin.js" } @@ -12169,7 +12154,6 @@ "integrity": "sha512-8dD6FusOQSrpv9Z1rdNMdlSgQOIP880DHqnohobOmYLElGEqAL/JvxvuxZO16r4HtjTlfPRDC1hbvxC9dPN2nA==", "dev": true, "license": "MIT", - "peer": true, "bin": { "marked": "bin/marked.js" }, @@ -15077,7 +15061,6 @@ "dev": true, "inBundle": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -15992,7 +15975,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "devOptional": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -16214,7 +16196,6 @@ "integrity": "sha512-I7AIg5boAr5R0FFtJ6rCfD+LFsWHp81dolrFD8S79U9tb8Az2nGrJncnMSnys+bpQJfRUzqs9hnA81OAA3hCuQ==", "dev": true, "license": "MIT", - "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -16318,6 +16299,7 @@ "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "ansi-regex": "^5.0.1", "ansi-styles": "^5.0.0", @@ -16333,6 +16315,7 @@ "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=10" }, @@ -16561,7 +16544,6 @@ "integrity": "sha512-DGrYcCWK7tvYMnWh79yrPHt+vdx9tY+1gPZa7nJQtO/p8bLTDaHp4dzwEhQB7pZ4Xe3ok4XKuEPrVuc+wlpkmw==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=0.10.0" } @@ -16572,6 +16554,7 @@ "integrity": "sha512-ibrK8llX2a4eOskq1mXKu/TGZj9qzomO+sNfO98M6d9zIPOEhlBkMkBUBLd1vgS0gQsLDBzA+8jJBVXDnfHmJg==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -16584,14 +16567,16 @@ "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.27.0.tgz", "integrity": "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/react-is": { "version": "17.0.2", "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/react-reconciler": { "version": "0.32.0", @@ -17083,7 +17068,6 @@ "integrity": "sha512-g7RssbTAbir1k/S7uSwSVZFfFXwpomUB9Oas0+xi9KStSCmeDXcA7rNhiskjLqvUe/Evhx8fVCT16OSa34eM5g==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@semantic-release/commit-analyzer": "^13.0.0-beta.1", "@semantic-release/error": "^4.0.0", @@ -18616,7 +18600,6 @@ "integrity": "sha512-yyxBKfORQ7LuRt/BQKBXrpcq59ZvSW0XxwfjAt3w2/8PmdxaFzijtMhTawprSHhpzeM5BgU2hXHG3lklIERZXg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -18766,7 +18749,6 @@ "integrity": "sha512-CWBzXQrc/qOkhidw1OzBTQuYRbfyxDXJMVJ1XNwUHGROVmuaeiEm3OslpZ1RV96d7SKKjZKrSJu3+t/xlw3R9A==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -18886,7 +18868,6 @@ "dev": true, "hasInstallScript": true, "license": "MIT", - "peer": true, "dependencies": { "napi-postinstall": "^0.3.0" }, @@ -19000,7 +18981,6 @@ "integrity": "sha512-cZn6NDFE7wdTpINgs++ZJ4N49W2vRp8LCKrn3Ob1kYNtOo21vfDoaV5GzBfLU4MovSAB8uNRm4jgzVQZ+mBzPQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.4.4", @@ -19099,7 +19079,6 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -19635,7 +19614,6 @@ "integrity": "sha512-PEIGCY5tSlUt50cqyMXfCzX+oOPqN0vuGqWzbcJ2xvnkzkq46oOpz7dQaTDBdfICb4N14+GARUDw2XV2N4tvzg==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10.0.0" }, @@ -19710,7 +19688,6 @@ "integrity": "sha512-lcYcMxX2PO9XMGvAJkJ3OsNMw+/7FKes7/hgerGUYWIoWu5j/+YQqcZr5JnPZWzOsEBgMbSbiSTn/dv/69Mkpw==", "dev": true, "license": "ISC", - "peer": true, "bin": { "yaml": "bin.mjs" }, @@ -19840,7 +19817,6 @@ "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "dev": true, "license": "MIT", - "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/extensions/cli/src/stream/mcp-headless.test.ts b/extensions/cli/src/stream/mcp-headless.test.ts index adf56cf8296..fb441f2e5df 100644 --- a/extensions/cli/src/stream/mcp-headless.test.ts +++ b/extensions/cli/src/stream/mcp-headless.test.ts @@ -328,4 +328,490 @@ describe("MCP tool execution permission in headless mode", () => { expect(result.approved).toBe(false); expect(result.denialReason).toBe("policy"); }); + + test("should approve built-in tools in headless mode regardless of allowHeadless", async () => { + const builtInTool: Tool = { + name: "Read", + displayName: "Read", + description: "Read a file", + parameters: { type: "object", properties: {} }, + run: async () => "result", + isBuiltIn: true, + allowHeadless: false, // Should not affect built-in tools + }; + const toolCall: PreprocessedToolCall = { + id: "test-id", + name: "Read", + arguments: {}, + argumentsStr: "{}", + startNotified: false, + tool: builtInTool, + }; + const permissions = { policies: [] }; + + const result = await checkToolPermissionApproval( + permissions, + toolCall, + undefined, + true, // isHeadless + ); + + // Built-in tools should be approved in headless mode + expect(result.approved).toBe(true); + }); +}); + +describe("MCP tool edge cases and error handling", () => { + beforeEach(() => { + // Clean up service container state before each test + Object.values(SERVICE_NAMES).forEach((service) => { + (serviceContainer as any).services.delete(service); + (serviceContainer as any).factories.delete(service); + (serviceContainer as any).dependencies.delete(service); + }); + }); + + test("should handle disconnected MCP servers gracefully", async () => { + await initializeServices({ headless: true }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [ + { + config: { + name: "disconnected-server", + command: "npx", + args: ["test"], + allowHeadless: true, + }, + status: "disconnected", // Server is disconnected + tools: [ + { + name: "mcp__test__tool", + description: "Test tool", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + ], + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + const tools = await getRequestTools(true); + const toolNames = tools.map((t) => t.function.name); + + // Should still enumerate tools from disconnected server + // (enumeration happens at config time, not runtime) + expect(toolNames).toContain("mcp__test__tool"); + }); + + test("should handle empty connections array", async () => { + await initializeServices({ headless: true }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [], // No connections + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + const tools = await getRequestTools(true); + + // Should not throw, just return built-in tools + expect(tools.length).toBeGreaterThan(0); + expect(tools.every((t) => t.function.name.startsWith("mcp__") === false)); + }); + + test("should handle MCP server with empty tools array", async () => { + await initializeServices({ headless: true }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [ + { + config: { + name: "empty-server", + command: "npx", + args: ["test"], + allowHeadless: true, + }, + status: "connected", + tools: [], // No tools available + prompts: [], + warnings: [], + }, + ], + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + const tools = await getRequestTools(true); + + // Should not throw, just skip this server + expect(tools).toBeDefined(); + }); + + test("should exclude MCP tools when allowHeadless is explicitly false", async () => { + await initializeServices({ headless: true }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [ + { + config: { + name: "test-server", + command: "npx", + args: ["test"], + allowHeadless: false, // Explicitly set to false + }, + status: "connected", + tools: [ + { + name: "mcp__test__search", + description: "Search tool", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + ], + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + const tools = await getRequestTools(true); + const toolNames = tools.map((t) => t.function.name); + + // Should be excluded when explicitly false + expect(toolNames).not.toContain("mcp__test__search"); + }); +}); + +describe("MCP tool permission policy interactions", () => { + function createMockToolCall( + toolName: string, + allowHeadless?: boolean, + ): PreprocessedToolCall { + const tool: Tool = { + name: toolName, + displayName: toolName, + description: "Test tool", + parameters: { type: "object", properties: {} }, + run: async () => "result", + isBuiltIn: false, + ...(allowHeadless !== undefined ? { allowHeadless } : {}), + }; + return { + id: "test-id", + name: toolName, + arguments: {}, + argumentsStr: "{}", + startNotified: false, + tool, + }; + } + + test("should respect wildcard exclude policies over allowHeadless", async () => { + const toolCall = createMockToolCall("mcp__test__search", true); + // Wildcard exclude for all MCP tools + const permissions = { + policies: [{ tool: "mcp__*", permission: "exclude" as const }], + }; + + const result = await checkToolPermissionApproval( + permissions, + toolCall, + undefined, + true, + ); + + // Wildcard exclude should take precedence + expect(result.approved).toBe(false); + expect(result.denialReason).toBe("policy"); + }); + + test("should allow specific tool even with wildcard ask policy", async () => { + const toolCall = createMockToolCall("mcp__test__search", true); + // More specific allow should take precedence over wildcard ask + const permissions = { + policies: [ + { tool: "mcp__test__search", permission: "allow" as const }, + { tool: "mcp__*", permission: "ask" as const }, + ], + }; + + const result = await checkToolPermissionApproval( + permissions, + toolCall, + undefined, + true, + ); + + // Specific allow should take precedence + expect(result.approved).toBe(true); + }); + + test("should apply allowHeadless with wildcard ask policy", async () => { + const toolCall = createMockToolCall("mcp__test__search", true); + // Wildcard ask for all MCP tools + const permissions = { + policies: [{ tool: "mcp__*", permission: "ask" as const }], + }; + + const result = await checkToolPermissionApproval( + permissions, + toolCall, + undefined, + true, // headless + ); + + // allowHeadless should upgrade "ask" to "allow" in headless mode + expect(result.approved).toBe(true); + }); + + test("should deny wildcard ask without allowHeadless in headless mode", async () => { + const toolCall = createMockToolCall("mcp__test__search", false); + // Wildcard ask for all MCP tools + const permissions = { + policies: [{ tool: "mcp__*", permission: "ask" as const }], + }; + + const result = await checkToolPermissionApproval( + permissions, + toolCall, + undefined, + true, // headless + ); + + // Without allowHeadless, "ask" should be denied in headless + expect(result.approved).toBe(false); + expect(result.denialReason).toBe("policy"); + }); + + test("should handle argumentMatches with allowHeadless", async () => { + const toolCall = createMockToolCall("mcp__test__search", true); + toolCall.arguments = { query: "safe search" }; + + // Policy that matches specific arguments + const permissions = { + policies: [ + { + tool: "mcp__test__search", + permission: "allow" as const, + argumentMatches: { query: "safe search" }, + }, + ], + }; + + const result = await checkToolPermissionApproval( + permissions, + toolCall, + undefined, + true, + ); + + // Should be allowed because arguments match + expect(result.approved).toBe(true); + }); +}); + +describe("MCP tool integration tests", () => { + beforeEach(() => { + Object.values(SERVICE_NAMES).forEach((service) => { + (serviceContainer as any).services.delete(service); + (serviceContainer as any).factories.delete(service); + (serviceContainer as any).dependencies.delete(service); + }); + }); + + test("should correctly mix built-in and MCP tools in headless mode", async () => { + await initializeServices({ headless: true }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [ + { + config: { + name: "allowed-server", + command: "npx", + args: ["test"], + allowHeadless: true, + }, + status: "connected", + tools: [ + { + name: "mcp__allowed__search", + description: "Allowed search", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + { + config: { + name: "blocked-server", + command: "npx", + args: ["test"], + allowHeadless: false, + }, + status: "connected", + tools: [ + { + name: "mcp__blocked__write", + description: "Blocked write", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + ], + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + const tools = await getRequestTools(true); + const toolNames = tools.map((t) => t.function.name); + + // Should have built-in tools + expect(toolNames).toContain("Read"); + expect(toolNames).toContain("Write"); + expect(toolNames).toContain("List"); + + // Should have allowed MCP tool + expect(toolNames).toContain("mcp__allowed__search"); + + // Should NOT have blocked MCP tool + expect(toolNames).not.toContain("mcp__blocked__write"); + }); + + test("should preserve tool order when filtering by allowHeadless", async () => { + await initializeServices({ headless: true }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [ + { + config: { + name: "server1", + command: "npx", + args: ["test"], + allowHeadless: true, + }, + status: "connected", + tools: [ + { + name: "mcp__server1__tool_a", + description: "Tool A", + inputSchema: { type: "object", properties: {} }, + }, + { + name: "mcp__server1__tool_b", + description: "Tool B", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + { + config: { + name: "server2", + command: "npx", + args: ["test"], + allowHeadless: false, + }, + status: "connected", + tools: [ + { + name: "mcp__server2__tool_c", + description: "Tool C", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + ], + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + const tools = await getRequestTools(true); + const mcpTools = tools.filter((t) => t.function.name.startsWith("mcp__")); + const mcpToolNames = mcpTools.map((t) => t.function.name); + + // Should maintain relative order of allowed tools + expect(mcpToolNames).toEqual([ + "mcp__server1__tool_a", + "mcp__server1__tool_b", + ]); + }); + + test("should handle transition from interactive to headless mode", async () => { + // Start in interactive mode + await initializeServices({ headless: false }); + + const mockMcpState: MCPServiceState = { + mcpService: null, + connections: [ + { + config: { + name: "test-server", + command: "npx", + args: ["test"], + allowHeadless: false, + }, + status: "connected", + tools: [ + { + name: "mcp__test__tool", + description: "Test tool", + inputSchema: { type: "object", properties: {} }, + }, + ], + prompts: [], + warnings: [], + }, + ], + tools: [], + prompts: [], + }; + + const mcpService = await serviceContainer.get(SERVICE_NAMES.MCP); + (mcpService as any).connections = mockMcpState.connections; + + // In interactive mode, tool should be available + const interactiveTools = await getRequestTools(false); + const interactiveToolNames = interactiveTools.map((t) => t.function.name); + expect(interactiveToolNames).toContain("mcp__test__tool"); + + // In headless mode, same tool should be excluded + const headlessTools = await getRequestTools(true); + const headlessToolNames = headlessTools.map((t) => t.function.name); + expect(headlessToolNames).not.toContain("mcp__test__tool"); + }); }); diff --git a/package-lock.json b/package-lock.json index a4b2f971428..fa5611dbe04 100644 --- a/package-lock.json +++ b/package-lock.json @@ -380,7 +380,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -1260,7 +1259,6 @@ "deprecated": "This version is no longer supported. Please see https://eslint.org/version-support for other options.", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", @@ -3472,7 +3470,6 @@ "integrity": "sha512-QQtaxnoDJeAkDvDKWCLiwIXkTgRhwYDEQCghU9Z6q03iyek/rxRh/2lC3HB7P8sWT2xC/y5JDctPLBIGzHKbhw==", "dev": true, "license": "MIT", - "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -4412,7 +4409,6 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver"