diff --git a/manifest.template.json b/manifest.template.json index e1e387e1..65528b4c 100644 --- a/manifest.template.json +++ b/manifest.template.json @@ -1,6 +1,6 @@ { "$schema": "../../dist/mcpb-manifest.schema.json", - "manifest_version": "0.1", + "manifest_version": "0.3", "name": "desktop-commander", "display_name": "Desktop Commander", "version": "{{VERSION}}", @@ -18,6 +18,7 @@ "homepage": "https://github.com/wonderwhy-er/DesktopCommanderMCP", "documentation": "https://github.com/wonderwhy-er/DesktopCommanderMCP/blob/main/FAQ.md", "support": "https://github.com/wonderwhy-er/DesktopCommanderMCP/issues", + "privacy_policies": ["https://legal.desktopcommander.app/privacy_desktop_commander_mcp"], "icon": "icon.png", "server": { "type": "node", diff --git a/scripts/build-mcpb.cjs b/scripts/build-mcpb.cjs index 794dd307..0a96c7bc 100755 --- a/scripts/build-mcpb.cjs +++ b/scripts/build-mcpb.cjs @@ -106,7 +106,8 @@ filesToCopy.forEach(file => { } }); -// Step 6: Create package.json in bundle with production dependencies +// Step 6: Create package.json in bundle with production dependencies from main package.json +// This ensures MCPB bundle always has the same dependencies as the npm package const bundlePackageJson = { name: manifest.name, version: manifest.version, @@ -115,27 +116,7 @@ const bundlePackageJson = { author: manifest.author, license: manifest.license, repository: manifest.repository, - dependencies: { - "@modelcontextprotocol/sdk": "^1.9.0", - "@opendocsg/pdf2md": "^0.2.2", - "@vscode/ripgrep": "^1.15.9", - "cross-fetch": "^4.1.0", - "exceljs": "^4.4.0", - "fastest-levenshtein": "^1.0.16", - "file-type": "^21.1.1", - "glob": "^10.3.10", - "isbinaryfile": "^5.0.4", - "md-to-pdf": "^5.2.5", - "pdf-lib": "^1.17.1", - "remark": "^15.0.1", - "remark-gfm": "^4.0.1", - "remark-parse": "^11.0.0", - "sharp": "^0.34.5", - "unified": "^11.0.5", - "unpdf": "^1.4.0", - "zod": "^3.24.1", - "zod-to-json-schema": "^3.23.5" - } + dependencies: packageJson.dependencies // Use dependencies directly from package.json }; fs.writeFileSync( diff --git a/src/config-manager.ts b/src/config-manager.ts index c6c19937..bddba3ea 100644 --- a/src/config-manager.ts +++ b/src/config-manager.ts @@ -15,7 +15,7 @@ export interface ServerConfig { fileReadLineLimit?: number; // Default line limit for file read operations (changed from character-based) clientId?: string; // Unique client identifier for analytics currentClient?: ClientInfo; // Current connected client information - [key: string]: any; // Allow for arbitrary configuration keys + [key: string]: any; // Allow for arbitrary configuration keys (including abTest_* keys) } export interface ClientInfo { @@ -30,6 +30,7 @@ class ConfigManager { private configPath: string; private config: ServerConfig = {}; private initialized = false; + private _isFirstRun = false; // Track if this is the first run (config was just created) constructor() { // Get user's home directory @@ -56,9 +57,11 @@ class ConfigManager { // Load existing config const configData = await fs.readFile(this.configPath, 'utf8'); this.config = JSON.parse(configData); + this._isFirstRun = false; } catch (error) { // Config file doesn't exist, create default this.config = this.getDefaultConfig(); + this._isFirstRun = true; // This is a first run! await this.saveConfig(); } this.config['version'] = VERSION; @@ -221,6 +224,13 @@ class ConfigManager { await this.saveConfig(); return { ...this.config }; } + + /** + * Check if this is the first run (config file was just created) + */ + isFirstRun(): boolean { + return this._isFirstRun; + } } // Export singleton instance diff --git a/src/server.ts b/src/server.ts index 471df2f9..72864b2f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -56,6 +56,7 @@ import { trackToolCall } from './utils/trackTools.js'; import { usageTracker } from './utils/usageTracker.js'; import { processDockerPrompt } from './utils/dockerPrompt.js'; import { toolHistory } from './utils/toolHistory.js'; +import { handleWelcomePageOnboarding } from './utils/welcome-onboarding.js'; import { VERSION } from './version.js'; import { capture, capture_call_tool } from "./utils/capture.js"; @@ -130,6 +131,11 @@ server.setRequestHandler(InitializeRequestSchema, async (request: InitializeRequ // Defer client connection message until after initialization deferLog('info', `Client connected: ${currentClient.name} v${currentClient.version}`); + + // Welcome page for new claude-ai users (A/B test controlled) + if (currentClient.name === 'claude-ai' && !(global as any).disableOnboarding) { + await handleWelcomePageOnboarding(); + } } capture('run_server_mcp_initialized'); @@ -422,6 +428,11 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { ${PATH_GUIDANCE} ${CMD_PREFIX_DESCRIPTION}`, inputSchema: zodToJsonSchema(CreateDirectoryArgsSchema), + annotations: { + title: "Create Directory", + readOnlyHint: false, + destructiveHint: false, + }, }, { name: "list_directory", @@ -561,6 +572,10 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { ${PATH_GUIDANCE} ${CMD_PREFIX_DESCRIPTION}`, inputSchema: zodToJsonSchema(StartSearchArgsSchema), + annotations: { + title: "Start Search", + readOnlyHint: true, + }, }, { name: "get_more_search_results", @@ -606,6 +621,11 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { ${CMD_PREFIX_DESCRIPTION}`, inputSchema: zodToJsonSchema(StopSearchArgsSchema), + annotations: { + title: "Stop Search", + readOnlyHint: false, + destructiveHint: false, + }, }, { name: "list_searches", @@ -1022,6 +1042,11 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { ${CMD_PREFIX_DESCRIPTION}`, inputSchema: zodToJsonSchema(GiveFeedbackArgsSchema), + annotations: { + title: "Give Feedback", + readOnlyHint: false, + openWorldHint: true, + }, }, { name: "get_prompts", @@ -1049,6 +1074,10 @@ server.setRequestHandler(ListToolsRequestSchema, async () => { ${CMD_PREFIX_DESCRIPTION}`, inputSchema: zodToJsonSchema(GetPromptsArgsSchema), + annotations: { + title: "Get Prompts", + readOnlyHint: true, + }, } ]; diff --git a/src/utils/ab-test.ts b/src/utils/ab-test.ts new file mode 100644 index 00000000..40c09337 --- /dev/null +++ b/src/utils/ab-test.ts @@ -0,0 +1,109 @@ +import { configManager } from '../config-manager.js'; +import { featureFlagManager } from './feature-flags.js'; + +/** + * A/B Test controlled feature flags + * + * Experiments are defined in remote feature flags JSON: + * { + * "flags": { + * "experiments": { + * "OnboardingPreTool": { + * "variants": ["noOnboardingPage", "showOnboardingPage"] + * } + * } + * } + * } + * + * Usage: + * if (await hasFeature('showOnboardingPage')) { ... } + */ + +interface Experiment { + variants: string[]; +} + +// Cache for variant assignments (loaded once per session) +const variantCache: Record = {}; + +/** + * Get experiments config from feature flags + */ +function getExperiments(): Record { + return featureFlagManager.get('experiments', {}); +} + +/** + * Get user's variant for an experiment (cached, deterministic) + */ +async function getVariant(experimentName: string): Promise { + const experiments = getExperiments(); + const experiment = experiments[experimentName]; + if (!experiment?.variants?.length) return null; + + // Check cache + if (variantCache[experimentName]) { + return variantCache[experimentName]; + } + + // Check persisted assignment + const configKey = `abTest_${experimentName}`; + const existing = await configManager.getValue(configKey); + + if (existing && experiment.variants.includes(existing)) { + variantCache[experimentName] = existing; + return existing; + } + + // New assignment based on clientId + const clientId = await configManager.getValue('clientId') || ''; + const hash = hashCode(clientId + experimentName); + const variantIndex = hash % experiment.variants.length; + const variant = experiment.variants[variantIndex]; + + await configManager.setValue(configKey, variant); + variantCache[experimentName] = variant; + return variant; +} + +/** + * Check if a feature (variant name) is enabled for current user + */ +export async function hasFeature(featureName: string): Promise { + const experiments = getExperiments(); + if (!experiments || typeof experiments !== 'object') return false; + + for (const [expName, experiment] of Object.entries(experiments)) { + if (experiment?.variants?.includes(featureName)) { + const variant = await getVariant(expName); + return variant === featureName; + } + } + return false; +} + +/** + * Get all A/B test assignments for analytics (reads from config) + */ +export async function getABTestAssignments(): Promise> { + const experiments = getExperiments(); + const assignments: Record = {}; + + for (const expName of Object.keys(experiments)) { + const configKey = `abTest_${expName}`; + const variant = await configManager.getValue(configKey); + if (variant) { + assignments[`ab_${expName}`] = variant; + } + } + return assignments; +} + +function hashCode(str: string): number { + let hash = 0; + for (let i = 0; i < str.length; i++) { + hash = ((hash << 5) - hash) + str.charCodeAt(i); + hash |= 0; + } + return Math.abs(hash); +} diff --git a/src/utils/capture.ts b/src/utils/capture.ts index e819e360..9c511ddf 100644 --- a/src/utils/capture.ts +++ b/src/utils/capture.ts @@ -99,6 +99,12 @@ export const captureBase = async (captureURL: string, event: string, properties? }; } + // Track if user saw onboarding page + const sawOnboardingPage = await configManager.getValue('sawOnboardingPage'); + if (sawOnboardingPage !== undefined) { + clientContext = { ...clientContext, saw_onboarding_page: sawOnboardingPage }; + } + // Create a deep copy of properties to avoid modifying the original objects // This ensures we don't alter error objects that are also returned to the AI let sanitizedProperties; diff --git a/src/utils/open-browser.ts b/src/utils/open-browser.ts new file mode 100644 index 00000000..efddb8d4 --- /dev/null +++ b/src/utils/open-browser.ts @@ -0,0 +1,46 @@ +import { execFile, spawn } from 'child_process'; +import os from 'os'; +import { logToStderr } from './logger.js'; + +/** + * Open a URL in the default browser (cross-platform) + * Uses execFile/spawn with args array to avoid shell injection + */ +export async function openBrowser(url: string): Promise { + const platform = os.platform(); + + return new Promise((resolve, reject) => { + const callback = (error: Error | null) => { + if (error) { + logToStderr('error', `Failed to open browser: ${error.message}`); + reject(error); + } else { + logToStderr('info', `Opened browser to: ${url}`); + resolve(); + } + }; + + switch (platform) { + case 'darwin': + execFile('open', [url], callback); + break; + case 'win32': + // Windows 'start' is a shell builtin, use spawn with shell but pass URL as separate arg + spawn('cmd', ['/c', 'start', '', url], { shell: false }).on('close', (code) => { + code === 0 ? resolve() : reject(new Error(`Exit code ${code}`)); + }); + break; + default: + execFile('xdg-open', [url], callback); + break; + } + }); +} + +/** + * Open the Desktop Commander welcome page + */ +export async function openWelcomePage(): Promise { + const url = 'https://desktopcommander.app/welcome/'; + await openBrowser(url); +} diff --git a/src/utils/welcome-onboarding.ts b/src/utils/welcome-onboarding.ts new file mode 100644 index 00000000..a6d9db8b --- /dev/null +++ b/src/utils/welcome-onboarding.ts @@ -0,0 +1,40 @@ +import { configManager } from '../config-manager.js'; +import { hasFeature } from './ab-test.js'; +import { openWelcomePage } from './open-browser.js'; +import { logToStderr } from './logger.js'; + +/** + * Handle welcome page display for new users (A/B test controlled) + * + * Only shows to: + * 1. New users (first run - config was just created) + * 2. Users in the 'showOnboardingPage' A/B variant + * 3. Haven't seen it yet + */ +export async function handleWelcomePageOnboarding(): Promise { + // Only for brand new users (config just created) + if (!configManager.isFirstRun()) { + return; + } + + // Check A/B test assignment + const shouldShow = await hasFeature('showOnboardingPage'); + if (!shouldShow) { + logToStderr('debug', 'Welcome page skipped (A/B: noOnboardingPage)'); + return; + } + + // Double-check not already shown (safety) + const alreadyShown = await configManager.getValue('sawOnboardingPage'); + if (alreadyShown) { + return; + } + + try { + await openWelcomePage(); + await configManager.setValue('sawOnboardingPage', true); + logToStderr('info', 'Welcome page opened'); + } catch (e) { + logToStderr('warning', `Failed to open welcome page: ${e instanceof Error ? e.message : e}`); + } +} diff --git a/test/ab-test.test.js b/test/ab-test.test.js new file mode 100644 index 00000000..20c9a957 --- /dev/null +++ b/test/ab-test.test.js @@ -0,0 +1,233 @@ +/** + * Unit tests for A/B test feature flag system + * Tests that missing/empty experiments config doesn't break anything + */ + +import assert from 'assert'; + +// Mock the dependencies before importing ab-test +let mockExperiments = {}; +let mockConfigValues = {}; + +// Mock featureFlagManager +const mockFeatureFlagManager = { + get: (key, defaultValue) => { + if (key === 'experiments') return mockExperiments; + return defaultValue; + } +}; + +// Mock configManager +const mockConfigManager = { + getValue: async (key) => mockConfigValues[key], + setValue: async (key, value) => { mockConfigValues[key] = value; } +}; + +// We need to test the logic directly since we can't easily mock ES modules +// Recreate the core functions with injected dependencies + +function getExperiments() { + return mockFeatureFlagManager.get('experiments', {}); +} + +function hashCode(str) { + let hash = 0; + for (let i = 0; i < str.length; i++) { + hash = ((hash << 5) - hash) + str.charCodeAt(i); + hash |= 0; + } + return Math.abs(hash); +} + +const variantCache = {}; + +async function getVariant(experimentName) { + const experiments = getExperiments(); + const experiment = experiments[experimentName]; + if (!experiment?.variants?.length) return null; + + if (variantCache[experimentName]) { + return variantCache[experimentName]; + } + + const configKey = `abTest_${experimentName}`; + const existing = await mockConfigManager.getValue(configKey); + + if (existing && experiment.variants.includes(existing)) { + variantCache[experimentName] = existing; + return existing; + } + + const clientId = await mockConfigManager.getValue('clientId') || ''; + const hash = hashCode(clientId + experimentName); + const variantIndex = hash % experiment.variants.length; + const variant = experiment.variants[variantIndex]; + + await mockConfigManager.setValue(configKey, variant); + variantCache[experimentName] = variant; + return variant; +} + +async function hasFeature(featureName) { + const experiments = getExperiments(); + if (!experiments || typeof experiments !== 'object') return false; + + for (const [expName, experiment] of Object.entries(experiments)) { + if (experiment?.variants?.includes(featureName)) { + const variant = await getVariant(expName); + return variant === featureName; + } + } + return false; +} + +// Clear state between tests +function resetState() { + mockExperiments = {}; + mockConfigValues = {}; + Object.keys(variantCache).forEach(k => delete variantCache[k]); +} + +// Test runner +async function runTests() { + let passed = 0; + let failed = 0; + + const test = async (name, fn) => { + resetState(); + try { + await fn(); + console.log(`โœ… ${name}`); + passed++; + } catch (e) { + console.log(`โŒ ${name}`); + console.log(` Error: ${e.message}`); + failed++; + } + }; + + console.log('\n๐Ÿงช A/B Test Feature Flag Tests\n'); + + // Test 1: No experiments at all + await test('hasFeature returns false when no experiments exist', async () => { + mockExperiments = {}; + const result = await hasFeature('showOnboardingPage'); + assert.strictEqual(result, false); + }); + + // Test 2: Experiments is undefined/null + await test('hasFeature returns false when experiments is undefined', async () => { + mockExperiments = undefined; + const result = await hasFeature('showOnboardingPage'); + assert.strictEqual(result, false); + }); + + // Test 3: Empty experiments object + await test('hasFeature returns false with empty experiments object', async () => { + mockExperiments = {}; + const result = await hasFeature('anyFeature'); + assert.strictEqual(result, false); + }); + + // Test 4: Experiment exists but variants array is empty + await test('hasFeature returns false when experiment has empty variants', async () => { + mockExperiments = { + 'TestExp': { variants: [] } + }; + const result = await hasFeature('showOnboardingPage'); + assert.strictEqual(result, false); + }); + + // Test 5: Experiment exists but variants is undefined + await test('hasFeature returns false when variants is undefined', async () => { + mockExperiments = { + 'TestExp': {} + }; + const result = await hasFeature('showOnboardingPage'); + assert.strictEqual(result, false); + }); + + // Test 6: Feature not in any experiment + await test('hasFeature returns false for unknown feature', async () => { + mockExperiments = { + 'OnboardingPreTool': { + variants: ['noOnboardingPage', 'showOnboardingPage'] + } + }; + const result = await hasFeature('unknownFeature'); + assert.strictEqual(result, false); + }); + + // Test 7: Feature exists, user assigned to it + await test('hasFeature returns true when user is assigned to that variant', async () => { + mockExperiments = { + 'OnboardingPreTool': { + variants: ['noOnboardingPage', 'showOnboardingPage'] + } + }; + mockConfigValues = { 'abTest_OnboardingPreTool': 'showOnboardingPage' }; + const result = await hasFeature('showOnboardingPage'); + assert.strictEqual(result, true); + }); + + // Test 8: Feature exists, user assigned to different variant + await test('hasFeature returns false when user is assigned to different variant', async () => { + mockExperiments = { + 'OnboardingPreTool': { + variants: ['noOnboardingPage', 'showOnboardingPage'] + } + }; + mockConfigValues = { 'abTest_OnboardingPreTool': 'noOnboardingPage' }; + const result = await hasFeature('showOnboardingPage'); + assert.strictEqual(result, false); + }); + + // Test 9: New user gets deterministic assignment + await test('new user gets deterministic variant assignment based on clientId', async () => { + mockExperiments = { + 'OnboardingPreTool': { + variants: ['noOnboardingPage', 'showOnboardingPage'] + } + }; + mockConfigValues = { clientId: 'test-client-123' }; + + const result1 = await hasFeature('showOnboardingPage'); + const result2 = await hasFeature('noOnboardingPage'); + + // One must be true, one must be false + assert.strictEqual(result1 !== result2, true, 'User should be in exactly one variant'); + + // Check it was persisted + const persisted = mockConfigValues['abTest_OnboardingPreTool']; + assert.ok(persisted, 'Assignment should be persisted to config'); + assert.ok(['noOnboardingPage', 'showOnboardingPage'].includes(persisted)); + }); + + // Test 10: Malformed experiment data doesn't crash + await test('malformed experiment data does not throw', async () => { + mockExperiments = { + 'BadExp1': null, + 'BadExp2': 'not an object', + 'BadExp3': { variants: 'not an array' }, + 'GoodExp': { variants: ['a', 'b'] } + }; + + // Should not throw + const result = await hasFeature('a'); + // Result depends on assignment, but shouldn't crash + assert.ok(typeof result === 'boolean'); + }); + + // Summary + console.log(`\n๐Ÿ“Š Results: ${passed} passed, ${failed} failed\n`); + + return failed === 0; +} + +// Run tests +runTests().then(success => { + process.exit(success ? 0 : 1); +}).catch(err => { + console.error('Test runner error:', err); + process.exit(1); +});