diff --git a/BUG_FIX_SUMMARY.md b/BUG_FIX_SUMMARY.md deleted file mode 100644 index 4f2c4916..00000000 --- a/BUG_FIX_SUMMARY.md +++ /dev/null @@ -1,106 +0,0 @@ -# ๐ŸŽ‰ BUG FIX COMPLETE: read_process_output on Completed Processes - -## โœ… Fix Applied - -### **Modified File:** `/src/tools/improved-process-tools.ts` - -### **Change Made:** -```typescript -// BEFORE (Buggy): -const session = terminalManager.getSession(pid); -if (!session) { - return { - content: [{ type: "text", text: `No active session found for PID ${pid}` }], - isError: true, - }; -} - -// AFTER (Fixed): -const session = terminalManager.getSession(pid); -if (!session) { - // Check if this is a completed session - const completedOutput = terminalManager.getNewOutput(pid); - if (completedOutput) { - return { - content: [{ - type: "text", - text: completedOutput - }], - }; - } - - // Neither active nor completed session found - return { - content: [{ type: "text", text: `No session found for PID ${pid}` }], - isError: true, - }; -} -``` - -## โœ… Verification Results - -### **Test Status:** -- โŒ **Before Fix:** `โœ— Test failed: ./test-read-completed-process.js (Exit code: 1)` -- โœ… **After Fix:** `โœ“ Test passed: ./test-read-completed-process.js (2702ms)` - -### **Your Exact Scenario Now Works:** -```bash -# 1. Start command with short timeout (returns before completion) -startProcess("sleep 1 && echo 'SUCCESS MESSAGE'", timeout: 500ms) -# โ†’ Returns immediately with PID - -# 2. Process completes in background, echo runs -# (1 second later...) - -# 3. Read from completed process -readProcessOutput(pid) -# โœ… NOW RETURNS: -# Process completed with exit code 0 -# Runtime: 1.0s -# Final output: -# SUCCESS MESSAGE -``` - -## ๐ŸŽฏ What the Fix Provides - -### **Before (Broken):** โŒ -- `read_process_output` on completed process โ†’ **Error:** "No active session found" -- Lost all output from completed processes -- Users confused about "missing" processes - -### **After (Fixed):** โœ… -- `read_process_output` on completed process โ†’ **Success:** Returns completion info -- **Exit code** (0 for success, non-zero for failure) -- **Runtime** (how long the process took) -- **Final output** (all stdout/stderr captured) -- **No more errors** for legitimately completed processes - -## ๐Ÿงช Test Coverage - -- โœ… **Delayed completion** (your scenario): Short timeout, process finishes later -- โœ… **Immediate completion**: Process finishes before timeout -- โœ… **Forced termination**: Process killed, can still read termination info -- โœ… **Integration**: Works with full test suite (23+ tests passing) - -## ๐Ÿš€ Impact - -### **User Experience:** -- No more confusing "No active session found" errors -- Can retrieve output from any process that ran, regardless of timing -- Better debugging capabilities with exit codes and runtime info - -### **API Consistency:** -- `read_process_output` now works intuitively for all process states -- Leverages existing `completedSessions` infrastructure -- Maintains backward compatibility - -### **Developer Benefits:** -- Easier to script process workflows -- Can implement "fire and forget" patterns -- Better error handling and debugging - -## ๐Ÿ“ Summary - -**The fix was simple but powerful:** When `read_process_output` can't find an active session, it now checks completed sessions before giving up. This leverages the existing `TerminalManager.getNewOutput()` capability that was already working perfectly but wasn't exposed through the API. - -**Result:** Your exact use case now works flawlessly! ๐ŸŽ‰ diff --git a/CODERABBIT_FIXES.md b/CODERABBIT_FIXES.md deleted file mode 100644 index 3201af40..00000000 --- a/CODERABBIT_FIXES.md +++ /dev/null @@ -1,93 +0,0 @@ -# โœ… CodeRabbit Feedback Addressed - Test Improvements - -## ๐Ÿ”ง Fixes Applied - -### 1. **Cross-Platform Compatibility** โœ… -**Issue:** Tests used shell-specific commands (`sleep`, `echo`) that fail on Windows - -**Fix Applied:** -```javascript -// BEFORE (Unix-only): -command: 'sleep 1 && echo "SUCCESS MESSAGE"' -command: 'echo "IMMEDIATE OUTPUT"' - -// AFTER (Cross-platform): -command: 'node -e "setTimeout(() => console.log(\'SUCCESS MESSAGE\'), 1000)"' -command: 'node -e "console.log(\'IMMEDIATE OUTPUT\')"' -``` - -**Benefits:** -- โœ… Works on Windows (cmd.exe, PowerShell) -- โœ… Works on macOS/Linux (bash, zsh, sh) -- โœ… No dependency on shell-specific commands -- โœ… Uses Node.js which is already required - -### 2. **Build Integration** โœ… -**Issue:** Test script didn't build first, could run against stale compiled code - -**Fix Applied:** -```json -// package.json BEFORE: -"test": "node test/run-all-tests.js" - -// package.json AFTER: -"test": "npm run build && node test/run-all-tests.js" -``` - -**Benefits:** -- โœ… Always uses latest compiled code -- โœ… Catches TypeScript compilation errors before testing -- โœ… Ensures dist/ directory exists -- โœ… Consistent CI/CD behavior - -### 3. **Verification** โœ… - -**Cross-Platform Test Results:** -```bash -# Node.js command works perfectly: -node -e "setTimeout(() => console.log('Cross-platform test works!'), 1000)" -# โœ… Returns: Cross-platform test works! -``` - -**Build-First Test Results:** -```bash -npm test -# โœ… Builds first, then runs all tests -# โœ… Test passed: ./test-read-completed-process.js (2752ms) -``` - -## ๐Ÿ“Š Test Status Summary - -### **Before Fixes:** -- โŒ Cross-platform issues (Windows compatibility) -- โŒ Potential stale code testing -- โŒ Missing build dependencies - -### **After Fixes:** -- โœ… **Cross-platform:** Works on Windows, macOS, Linux -- โœ… **Build integration:** Always tests latest code -- โœ… **Robust testing:** Proper CI/CD ready -- โœ… **Bug validation:** Confirms fix works universally - -## ๐ŸŽฏ Core Functionality Verified - -The main fix (read_process_output on completed processes) works perfectly: - -```javascript -// Your scenario - now cross-platform and robust: -startProcess('node -e "setTimeout(() => console.log(\'SUCCESS\'), 1000)"', timeout: 500ms) -// โ†’ Returns before output - -// Later... -readProcessOutput(pid) -// โœ… Returns: Process completed with exit code 0, Runtime: 1.0s, Final output: SUCCESS -``` - -## ๐Ÿš€ Ready for Production - -- โœ… **Cross-platform compatibility** -- โœ… **Proper build integration** -- โœ… **Robust test infrastructure** -- โœ… **Core bug fix validated** - -All CodeRabbit feedback has been addressed! ๐ŸŽ‰ diff --git a/FINAL_TEST_STATUS.md b/FINAL_TEST_STATUS.md deleted file mode 100644 index be9c6c3a..00000000 --- a/FINAL_TEST_STATUS.md +++ /dev/null @@ -1,70 +0,0 @@ -# โœ… Test Status Summary - -## Current Active Test โœ… -**`test-read-completed-process.js`** - The correct, proper test -- โœ… **Fails when bug exists** (original behavior) -- โœ… **Passes when bug is fixed** (current behavior) -- โœ… **Tests cross-platform** (uses Node.js commands) -- โœ… **Integrated with build** (npm test builds first) -- โœ… **Tests core functionality** (your exact use case) - -## Disabled/Temp Files ๐Ÿ—‚๏ธ -**Properly disabled (incorrect design):** -- `test-read-completed-process.js.disabled` - Bad test (passed when demonstrating bug) -- `test-echo-after-completion.js.disabled` - Bad test (passed when demonstrating bug) -- `test-direct-getNewOutput.js.disabled` - Debugging test - -**Moved to .temp (development artifacts):** -- `debug-quick.js.temp` - Quick debugging -- `demonstrate-fix.js.temp` - Demo script -- `test-format.js.temp` - Format testing - -## Test Integration Status โœ… - -### **Proper Test Results:** -```bash -cd /Users/fiberta/work/DesktopCommanderMCP && node test/test-read-completed-process.js -# โœ… All tests passed - read_process_output works on completed processes! -``` - -### **Full Test Suite:** -```bash -npm test -# โœ… Builds first, then runs all tests -# โœ… Test passed: ./test-read-completed-process.js (2752ms) -``` - -## What the Proper Test Validates โœ… - -### **Scenario 1: Delayed Completion** -```javascript -// Start with short timeout (returns before completion) -startProcess('node -e "setTimeout(() => console.log(\'SUCCESS MESSAGE\'), 1000)"', timeout: 500ms) - -// Wait for actual completion -await delay(2000) - -// Should be able to read completed process -readProcessOutput(pid) -// โœ… Returns: SUCCESS MESSAGE + completion info -``` - -### **Scenario 2: Immediate Completion** -```javascript -// Start immediate command -startProcess('node -e "console.log(\'IMMEDIATE OUTPUT\')"', timeout: 2000) - -// Should be able to read immediately completed process -readProcessOutput(pid) -// โœ… Returns: IMMEDIATE OUTPUT + completion info -``` - -## Summary ๐ŸŽฏ - -- โœ… **One proper test** that correctly validates the fix -- โœ… **Cross-platform compatible** (Node.js commands) -- โœ… **Proper test behavior** (fails with bug, passes with fix) -- โœ… **Integrated with build system** (npm test works) -- โœ… **Clean test directory** (temp/demo files moved aside) - -**The test infrastructure is now clean and proper!** ๐Ÿš€ diff --git a/PROPER_TEST_SUMMARY.md b/PROPER_TEST_SUMMARY.md deleted file mode 100644 index 379c20ec..00000000 --- a/PROPER_TEST_SUMMARY.md +++ /dev/null @@ -1,63 +0,0 @@ -# ๐ŸŽฏ SUCCESS: Proper Test Created for read_process_output Bug - -## What We Accomplished โœ… - -### 1. **Identified My Error** -- My initial tests were **backwards** - they passed when demonstrating the bug -- Proper tests should **FAIL when bug exists**, **PASS when bug is fixed** - -### 2. **Created Correct Test** -- `test-read-completed-process.js` - **FAILS with current code** โŒ -- Demonstrates exact scenario: short timeout, process completes, try to read echo -- Uses proper `assert()` statements that expect the fix to work - -### 3. **Verified Test Integration** -- Test is automatically discovered by `run-all-tests.js` -- Shows up as: `โœ— Test failed: ./test-read-completed-process.js (Exit code: 1)` -- **This is the desired behavior** - test fails until bug is fixed - -### 4. **Cleaned Up Bad Tests** -- Disabled the incorrectly designed tests (`.disabled` extension) -- Only the proper test remains active - -## Test Behavior ๐Ÿงช - -### **Current State (Bug Exists)** -```bash -cd /Users/fiberta/work/DesktopCommanderMCP && node test/run-all-tests.js -# Shows: โœ— Test failed: ./test-read-completed-process.js (Exit code: 1) -# This is CORRECT - test should fail when bug exists -``` - -### **After Fix (Bug Fixed)** -```bash -cd /Users/fiberta/work/DesktopCommanderMCP && node test/run-all-tests.js -# Will show: โœ… Test passed: ./test-read-completed-process.js -# This will confirm the fix works -``` - -## The Exact Test Scenario ๐Ÿ“‹ - -```javascript -// 1. Start command that completes after timeout -startProcess('sleep 1 && echo "SUCCESS MESSAGE"', timeout: 500ms) - -// 2. Wait for actual completion -await delay(2000) - -// 3. Try to read (should work when fixed) -readProcessOutput(pid) - -// 4. Assert success (currently fails, will pass when fixed) -assert(!readResult.isError, 'Should read from completed process') -assert(readResult.content[0].text.includes('SUCCESS MESSAGE'), 'Should get echo') -``` - -## Summary ๐ŸŽ‰ - -โœ… **Test correctly FAILS with current buggy code** -โœ… **Test will PASS when bug is fixed** -โœ… **Test captures your exact use case scenario** -โœ… **Test integrates with existing test suite** - -Perfect test-driven development setup! The test is ready to validate the fix. ๐Ÿš€ diff --git a/TEST_SUMMARY.md b/TEST_SUMMARY.md deleted file mode 100644 index 20a03132..00000000 --- a/TEST_SUMMARY.md +++ /dev/null @@ -1,102 +0,0 @@ -# Test Summary: read_process_output on Completed Processes - -## โœ… Tests Created and Verified - -### 1. **Comprehensive Test Suite** (`test-read-completed-process.js`) -- Tests 3 scenarios: delayed completion, immediate completion, forced termination -- Demonstrates current limitation across different process lifecycle states -- Will automatically validate the fix when implemented - -### 2. **Focused Echo Test** (`test-echo-after-completion.js`) -- **Perfect reproduction of your scenario:** - - Starts: `sleep 1 && echo "SUCCESS MESSAGE"` - - Uses 500ms timeout (returns before echo) - - Process completes, echo runs - - Calls `read_process_output` - - **Result: Cannot get the echo output** โŒ - -### 3. **Direct Infrastructure Test** (`test-direct-getNewOutput.js`) -- Confirms `TerminalManager.getNewOutput()` **ALREADY WORKS** for completed processes -- Proves the capability exists, just not exposed through `readProcessOutput` - -## ๐Ÿ” Key Findings - -### Current Behavior (Broken) โŒ -```bash -# Start process with short timeout -startProcess("sleep 1 && echo 'SUCCESS MESSAGE'", timeout: 500ms) -# โ†’ Returns immediately with PID, process continues running - -# Wait for completion... -# Process finishes, echo output is captured internally - -# Try to read -readProcessOutput(pid) -# โ†’ Error: "No active session found for PID" -# โ†’ Lost the "SUCCESS MESSAGE" output! -``` - -### Infrastructure Reality โœ… -```javascript -// This ALREADY WORKS: -terminalManager.getNewOutput(pid) -// โ†’ "Process completed with exit code 0\nRuntime: 1.0s\nFinal output:\nSUCCESS MESSAGE" -``` - -### The Problem ๐Ÿ› -`readProcessOutput` only checks active sessions: -```typescript -const session = terminalManager.getSession(pid); // Only active sessions -if (!session) { - return { error: "No active session found" }; // Fails here! -} -``` - -But it should ALSO check completed sessions: -```typescript -const session = terminalManager.getSession(pid); -if (session) { - // Handle active session... -} else { - // Check completed sessions - const completedOutput = terminalManager.getNewOutput(pid); - if (completedOutput) { - return { content: [{ type: "text", text: completedOutput }] }; - } - return { error: "No session found" }; -} -``` - -## ๐Ÿงช Test Results - -All tests **PASS** by confirming the current limitation exists: - -``` -โŒ CURRENT BEHAVIOR: No active session found for PID 16320 -โŒ Cannot read from completed process -โŒ Lost the "SUCCESS MESSAGE" echo output - -๐Ÿ”ง WHEN FIXED, should return: - Process completed with exit code 0 - Runtime: ~1.0s - Final output: SUCCESS MESSAGE -``` - -## ๐ŸŽฏ Perfect Test Case - -Your exact scenario is now captured in `test-echo-after-completion.js`: - -1. โœ… Command with small timeout that returns before completion -2. โœ… Process finishes and generates echo output -3. โœ… `read_process_output` called after completion -4. โœ… **Today: Gets no echo** (demonstrates bug) -5. โœ… **When fixed: Should get echo** (will validate fix) - -## ๐Ÿš€ Ready for Fix Implementation - -The tests are ready to: -- โœ… **Validate current bug** (all pass showing limitation) -- โœ… **Verify fix works** (will pass showing success when fixed) -- โœ… **Prevent regression** (will catch if bug reappears) - -The fix is simple and the tests prove the infrastructure already exists! diff --git a/improved-docker-detection.ts b/improved-docker-detection.ts deleted file mode 100644 index 18bcd7b8..00000000 --- a/improved-docker-detection.ts +++ /dev/null @@ -1,237 +0,0 @@ -import os from 'os'; -import fs from 'fs'; - -export interface ContainerInfo { - isContainer: boolean; - runtime?: 'docker' | 'podman' | 'containerd' | 'kubernetes' | 'lxc' | 'systemd-nspawn' | 'unknown'; - detectionMethods: string[]; - confidence: 'high' | 'medium' | 'low'; - details?: { - cgroupInfo?: string; - mountInfo?: string; - initProcess?: string; - containerEnv?: string; - }; -} - -/** - * Enhanced container detection with multiple methods and confidence scoring - */ -export function detectContainerEnvironment(): ContainerInfo { - const detectionMethods: string[] = []; - let runtime: ContainerInfo['runtime'] = undefined; - let confidence: ContainerInfo['confidence'] = 'low'; - const details: ContainerInfo['details'] = {}; - - // Method 1: Environment variable override (highest confidence) - if (process.env.MCP_CLIENT_DOCKER === 'true') { - detectionMethods.push('MCP_CLIENT_DOCKER env var'); - runtime = 'docker'; - confidence = 'high'; - details.containerEnv = 'MCP_CLIENT_DOCKER=true'; - } - - // Method 2: Standard container environment variables - const containerEnvVars = [ - 'DOCKER_CONTAINER', - 'CONTAINER', - 'KUBERNETES_SERVICE_HOST', - 'K8S_NODE_NAME' - ]; - - for (const envVar of containerEnvVars) { - if (process.env[envVar]) { - detectionMethods.push(`${envVar} env var`); - if (envVar.includes('KUBERNETES') || envVar.includes('K8S')) { - runtime = 'kubernetes'; - } else if (envVar.includes('DOCKER')) { - runtime = 'docker'; - } - confidence = 'high'; - details.containerEnv = `${envVar}=${process.env[envVar]}`; - } - } - - // Method 3: Check for container marker files - const containerFiles = [ - { path: '/.dockerenv', runtime: 'docker' as const }, - { path: '/.containerenv', runtime: 'podman' as const }, - { path: '/run/.containerenv', runtime: 'podman' as const } - ]; - - for (const file of containerFiles) { - if (fs.existsSync(file.path)) { - detectionMethods.push(`${file.path} file exists`); - runtime = file.runtime; - confidence = 'high'; - } - } - - // Method 4: Enhanced cgroup analysis (Linux only) - if (os.platform() === 'linux') { - try { - const cgroupPaths = ['/proc/1/cgroup', '/proc/self/cgroup']; - - for (const cgroupPath of cgroupPaths) { - if (fs.existsSync(cgroupPath)) { - const cgroup = fs.readFileSync(cgroupPath, 'utf8'); - details.cgroupInfo = cgroup; - - const containerIndicators = [ - { pattern: /docker/i, runtime: 'docker' as const }, - { pattern: /containerd/i, runtime: 'containerd' as const }, - { pattern: /kubepods/i, runtime: 'kubernetes' as const }, - { pattern: /libpod/i, runtime: 'podman' as const }, - { pattern: /lxc/i, runtime: 'lxc' as const }, - { pattern: /machine\.slice/i, runtime: 'systemd-nspawn' as const }, - { pattern: /\.scope$/m, runtime: 'unknown' as const } // Generic container scope - ]; - - for (const indicator of containerIndicators) { - if (indicator.pattern.test(cgroup)) { - detectionMethods.push(`cgroup ${indicator.runtime} pattern`); - if (!runtime || runtime === 'unknown') { - runtime = indicator.runtime; - } - confidence = confidence === 'low' ? 'medium' : confidence; - break; - } - } - break; - } - } - } catch (error) { - // cgroup files might not be accessible - } - } - - // Method 5: Mount information analysis (Linux only) - if (os.platform() === 'linux') { - try { - const mountInfo = fs.readFileSync('/proc/self/mountinfo', 'utf8'); - details.mountInfo = mountInfo.substring(0, 500); // First 500 chars for debugging - - // Look for overlay filesystem (common in containers) - if (mountInfo.includes('overlay')) { - detectionMethods.push('overlay filesystem detected'); - confidence = confidence === 'low' ? 'medium' : confidence; - } - - // Look for container-specific mount paths - const containerMountPatterns = [ - /\/var\/lib\/docker/, - /\/var\/lib\/containers/, - /\/run\/containerd/, - /\/var\/lib\/kubelet/ - ]; - - for (const pattern of containerMountPatterns) { - if (pattern.test(mountInfo)) { - detectionMethods.push(`container mount pattern: ${pattern.source}`); - confidence = confidence === 'low' ? 'medium' : confidence; - } - } - } catch (error) { - // /proc/self/mountinfo might not be accessible - } - } - - // Method 6: Init process analysis (Linux only) - if (os.platform() === 'linux') { - try { - const cmdline = fs.readFileSync('/proc/1/cmdline', 'utf8'); - details.initProcess = cmdline.replace(/\0/g, ' ').trim(); - - // Container runtimes often have specific init processes - const initPatterns = [ - { pattern: /runc/, runtime: 'docker' as const }, - { pattern: /crun/, runtime: 'podman' as const }, - { pattern: /containerd-shim/, runtime: 'containerd' as const }, - { pattern: /pause/, runtime: 'kubernetes' as const } - ]; - - for (const initPattern of initPatterns) { - if (initPattern.pattern.test(cmdline)) { - detectionMethods.push(`init process: ${initPattern.runtime}`); - if (!runtime || runtime === 'unknown') { - runtime = initPattern.runtime; - } - confidence = confidence === 'low' ? 'medium' : confidence; - } - } - } catch (error) { - // /proc/1/cmdline might not be accessible - } - } - - // Method 7: Hostname analysis - try { - const hostname = os.hostname(); - - // Docker containers often have hex-like hostnames - if (hostname && /^[a-f0-9]{12}$/.test(hostname)) { - detectionMethods.push('docker-style hostname pattern'); - if (!runtime) { - runtime = 'docker'; - } - confidence = confidence === 'low' ? 'medium' : confidence; - } - - // Kubernetes pods often have specific naming patterns - if (hostname && /-[a-z0-9]{5,10}-[a-z0-9]{5}$/.test(hostname)) { - detectionMethods.push('kubernetes-style hostname pattern'); - runtime = 'kubernetes'; - confidence = confidence === 'low' ? 'medium' : confidence; - } - } catch (error) { - // hostname not available - } - - // Method 8: Check for container-specific directories - const containerDirs = [ - '/var/run/docker.sock', - '/run/containerd/containerd.sock', - '/var/run/podman', - '/var/lib/kubelet' - ]; - - for (const dir of containerDirs) { - if (fs.existsSync(dir)) { - detectionMethods.push(`container socket/dir: ${dir}`); - confidence = confidence === 'low' ? 'medium' : confidence; - } - } - - const isContainer = detectionMethods.length > 0; - - return { - isContainer, - runtime, - detectionMethods, - confidence, - details - }; -} - -/** - * Simple boolean check for backward compatibility - */ -export function isRunningInContainer(): boolean { - return detectContainerEnvironment().isContainer; -} - -/** - * Get detailed container information for logging - */ -export function getContainerDetails(): string { - const info = detectContainerEnvironment(); - - if (!info.isContainer) { - return 'Not running in container'; - } - - const runtime = info.runtime || 'unknown'; - const methods = info.detectionMethods.join(', '); - - return `Container: ${runtime} (${info.confidence} confidence) - Methods: ${methods}`; -} diff --git a/process_read_analysis.md b/process_read_analysis.md deleted file mode 100644 index f01856bf..00000000 --- a/process_read_analysis.md +++ /dev/null @@ -1,119 +0,0 @@ -# Analysis: What Happens When read_process_output is Called on a Closed Process - -## Current Behavior - -When `read_process_output` is called on a process that has already exited/closed, the function returns an error: - -``` -No active session found for PID -``` - -## Root Cause - -The issue is in `/src/tools/improved-process-tools.ts` in the `readProcessOutput` function around line 110: - -```typescript -const session = terminalManager.getSession(pid); -if (!session) { - return { - content: [{ type: "text", text: `No active session found for PID ${pid}` }], - isError: true, - }; -} -``` - -This check only looks for **active** sessions using `getSession()`, which only checks the `sessions` Map. - -## The Problem - -However, when a process exits, the `TerminalManager` does two things: -1. **Removes** the session from `sessions` Map (active sessions) -2. **Adds** it to `completedSessions` Map (completed sessions) - -The `getNewOutput(pid)` method **can** actually return information about completed sessions: - -```typescript -getNewOutput(pid: number): string | null { - // First check active sessions - const session = this.sessions.get(pid); - if (session) { - const output = session.lastOutput; - session.lastOutput = ''; - return output; - } - - // Then check completed sessions โ† THIS PART WORKS! - const completedSession = this.completedSessions.get(pid); - if (completedSession) { - // Format completion message with exit code and runtime - const runtime = (completedSession.endTime.getTime() - completedSession.startTime.getTime()) / 1000; - return `Process completed with exit code ${completedSession.exitCode}\nRuntime: ${runtime}s\nFinal output:\n${completedSession.output}`; - } - - return null; -} -``` - -## Expected vs Actual Behavior - -### Expected: -When calling `read_process_output` on a completed process, it should return the final output and completion status. - -### Actual: -It returns an error saying "No active session found". - -## Demonstration - -```bash -# Start a quick process -start_process("echo 'Hello world'") -# Returns: Process started with PID 15707 -# Process immediately completes - -# Try to read from it -read_process_output(15707) -# Returns: Error - No active session found for PID 15707 -``` - -## The Fix - -The `readProcessOutput` function should be modified to handle completed sessions gracefully: - -```typescript -export async function readProcessOutput(args: unknown): Promise { - // ... validation code ... - - const { pid, timeout_ms = 5000 } = parsed.data; - - const session = terminalManager.getSession(pid); - - // Check if it's an active session - if (session) { - // ... existing logic for active sessions ... - } - - // If no active session, check for completed session - const completedOutput = terminalManager.getNewOutput(pid); - if (completedOutput) { - return { - content: [{ - type: "text", - text: completedOutput - }], - }; - } - - // Neither active nor completed - return { - content: [{ type: "text", text: `No session found for PID ${pid}` }], - isError: true, - }; -} -``` - -## Impact - -This change would make the behavior more intuitive and allow users to: -1. Read final output from processes that completed quickly -2. Get completion status and exit codes -3. Avoid confusion about "missing" processes that actually completed diff --git a/test_read_behavior.py b/test_read_behavior.py deleted file mode 100644 index b8f7e62b..00000000 --- a/test_read_behavior.py +++ /dev/null @@ -1,122 +0,0 @@ -#!/usr/bin/env python3 -""" -Test script to demonstrate read_process_output behavior on closed processes -""" - -import json -import subprocess -import time - -def call_mcp_tool(tool_name, params): - """Call an MCP tool and return the result""" - # This is a simplified example - in reality you'd use the MCP protocol - print(f"๐Ÿ”ง Calling {tool_name} with params: {params}") - - # Simulate the actual behavior we observed - if tool_name == "start_process": - return {"pid": 12345, "output": "Hello world\n"} - elif tool_name == "read_process_output" and params.get("pid") == 12345: - return {"error": "No active session found for PID 12345"} - - return {"error": "Unknown tool or params"} - -def demonstrate_current_behavior(): - """Demonstrate the current problematic behavior""" - print("=" * 60) - print("DEMONSTRATING CURRENT BEHAVIOR") - print("=" * 60) - - # Start a quick process - print("1. Starting a quick process (echo)") - result1 = call_mcp_tool("start_process", {"command": "echo 'Hello world'"}) - pid = result1.get("pid") - print(f" Result: PID {pid}, Output: {result1.get('output', '').strip()}") - - # Process has already completed at this point - print("\n2. Process completes immediately (echo is fast)") - print(" Process moves from active sessions to completed sessions") - - # Try to read from completed process - print(f"\n3. Attempting to read from completed process (PID {pid})") - result2 = call_mcp_tool("read_process_output", {"pid": pid}) - if "error" in result2: - print(f" โŒ Error: {result2['error']}") - else: - print(f" โœ… Success: {result2}") - -def demonstrate_proposed_fix(): - """Demonstrate how the proposed fix would work""" - print("\n" + "=" * 60) - print("PROPOSED FIX BEHAVIOR") - print("=" * 60) - - print("1. Starting a quick process (echo)") - print(" Result: PID 12345, Output: Hello world") - - print("\n2. Process completes immediately") - print(" Process moves from active sessions to completed sessions") - - print("\n3. Attempting to read from completed process (PID 12345)") - print(" โœ… Success: Process completed with exit code 0") - print(" Runtime: 0.1s") - print(" Final output:") - print(" Hello world") - -def show_code_analysis(): - """Show the code analysis""" - print("\n" + "=" * 60) - print("CODE ANALYSIS") - print("=" * 60) - - print(""" -The issue is in readProcessOutput function: - -CURRENT CODE: -```typescript -const session = terminalManager.getSession(pid); -if (!session) { - return { - content: [{ type: "text", text: `No active session found for PID ${pid}` }], - isError: true, - }; -} -``` - -PROBLEM: -- getSession() only checks active sessions -- When process completes, it's moved to completedSessions -- But readProcessOutput doesn't check completedSessions - -SOLUTION: -```typescript -const session = terminalManager.getSession(pid); -if (session) { - // Handle active session (existing logic) -} else { - // Check for completed session - const completedOutput = terminalManager.getNewOutput(pid); - if (completedOutput) { - return { content: [{ type: "text", text: completedOutput }] }; - } - return { content: [{ type: "text", text: `No session found for PID ${pid}` }], isError: true }; -} -``` -""") - -def main(): - demonstrate_current_behavior() - demonstrate_proposed_fix() - show_code_analysis() - - print("\n" + "=" * 60) - print("SUMMARY") - print("=" * 60) - print(""" -โŒ CURRENT: read_process_output fails on completed processes -โœ… PROPOSED: read_process_output returns completion info for completed processes - -This would make the API more user-friendly and intuitive! -""") - -if __name__ == "__main__": - main() diff --git a/track-installation.js b/track-installation.js deleted file mode 100644 index 7caf58e0..00000000 --- a/track-installation.js +++ /dev/null @@ -1,368 +0,0 @@ -#!/usr/bin/env node - -/** - * Installation Source Tracking Script - * Runs during npm install to detect how Desktop Commander was installed - * - * Debug logging can be enabled with: - * - DEBUG=desktop-commander npm install - * - DEBUG=* npm install - * - NODE_ENV=development npm install - * - DC_DEBUG=true npm install - */ - -import { randomUUID } from 'crypto'; -import * as https from 'https'; -import { platform } from 'os'; -import path from 'path'; -import { fileURLToPath } from 'url'; - -// Get current file directory -const __filename = fileURLToPath(import.meta.url); -const __dirname = path.dirname(__filename); - -// Debug logging utility - configurable via environment variables -const DEBUG_ENABLED = process.env.DEBUG === 'desktop-commander' || - process.env.DEBUG === '*' || - process.env.NODE_ENV === 'development' || - process.env.DC_DEBUG === 'true'; - -const debug = (...args) => { - if (DEBUG_ENABLED) { - console.log('[Desktop Commander Debug]', ...args); - } -}; - -const log = (...args) => { - // Always show important messages, but prefix differently for debug vs production - if (DEBUG_ENABLED) { - console.log('[Desktop Commander]', ...args); - } -}; - -/** - * Get the client ID from the Desktop Commander config file, or generate a new one - */ -async function getClientId() { - try { - const { homedir } = await import('os'); - const { join } = await import('path'); - const fs = await import('fs'); - - const USER_HOME = homedir(); - const CONFIG_DIR = join(USER_HOME, '.claude-server-commander'); - const CONFIG_FILE = join(CONFIG_DIR, 'config.json'); - - // Try to read existing config - if (fs.existsSync(CONFIG_FILE)) { - const configData = fs.readFileSync(CONFIG_FILE, 'utf8'); - const config = JSON.parse(configData); - if (config.clientId) { - debug(`Using existing clientId from config: ${config.clientId.substring(0, 8)}...`); - return config.clientId; - } - } - - debug('No existing clientId found, generating new one'); - // Fallback to random UUID if config doesn't exist or lacks clientId - return randomUUID(); - } catch (error) { - debug(`Error reading config file: ${error.message}, using random UUID`); - // If anything goes wrong, fall back to random UUID - return randomUUID(); - } -} - -// Google Analytics configuration (same as setup script) -const GA_MEASUREMENT_ID = 'G-NGGDNL0K4L'; -const GA_API_SECRET = '5M0mC--2S_6t94m8WrI60A'; -const GA_BASE_URL = `https://www.google-analytics.com/mp/collect?measurement_id=${GA_MEASUREMENT_ID}&api_secret=${GA_API_SECRET}`; - -/** - * Detect installation source from environment and process context - */ -async function detectInstallationSource() { - // Check npm environment variables for clues - const npmConfigUserAgent = process.env.npm_config_user_agent || ''; - const npmExecpath = process.env.npm_execpath || ''; - const npmCommand = process.env.npm_command || ''; - const npmLifecycleEvent = process.env.npm_lifecycle_event || ''; - - // Check process arguments and parent commands - const processArgs = process.argv.join(' '); - const processTitle = process.title || ''; - - debug('Installation source detection...'); - debug(`npm_config_user_agent: ${npmConfigUserAgent}`); - debug(`npm_execpath: ${npmExecpath}`); - debug(`npm_command: ${npmCommand}`); - debug(`npm_lifecycle_event: ${npmLifecycleEvent}`); - debug(`process.argv: ${processArgs}`); - debug(`process.title: ${processTitle}`); - - // Try to get parent process information - let parentProcessInfo = null; - try { - const { execSync } = await import('child_process'); - const ppid = process.ppid; - if (ppid && process.platform !== 'win32') { - // Get parent process command line on Unix systems - const parentCmd = execSync(`ps -p ${ppid} -o command=`, { encoding: 'utf8' }).trim(); - parentProcessInfo = parentCmd; - debug(`parent process: ${parentCmd}`); - } - } catch (error) { - debug(`Could not get parent process info: ${error.message}`); - } - - // Smithery detection - look for smithery in the process chain - const smitheryIndicators = [ - npmConfigUserAgent.includes('smithery'), - npmExecpath.includes('smithery'), - processArgs.includes('smithery'), - processArgs.includes('@smithery/cli'), - processTitle.includes('smithery'), - parentProcessInfo && parentProcessInfo.includes('smithery'), - parentProcessInfo && parentProcessInfo.includes('@smithery/cli') - ]; - - if (smitheryIndicators.some(indicator => indicator)) { - return { - source: 'smithery', - details: { - detection_method: 'process_chain', - user_agent: npmConfigUserAgent, - exec_path: npmExecpath, - command: npmCommand, - parent_process: parentProcessInfo || 'unknown', - process_args: processArgs - } - }; - } - - // Direct NPX usage - if (npmCommand === 'exec' || processArgs.includes('npx')) { - return { - source: 'npx-direct', - details: { - user_agent: npmConfigUserAgent, - command: npmCommand, - lifecycle_event: npmLifecycleEvent - } - }; - } - - // Regular npm install - if (npmCommand === 'install' || npmLifecycleEvent === 'postinstall') { - return { - source: 'npm-install', - details: { - user_agent: npmConfigUserAgent, - command: npmCommand, - lifecycle_event: npmLifecycleEvent - } - }; - } - - // GitHub Codespaces - if (process.env.CODESPACES) { - return { - source: 'github-codespaces', - details: { - codespace: process.env.CODESPACE_NAME || 'unknown' - } - }; - } - - // VS Code - if (process.env.VSCODE_PID || process.env.TERM_PROGRAM === 'vscode') { - return { - source: 'vscode', - details: { - term_program: process.env.TERM_PROGRAM, - vscode_pid: process.env.VSCODE_PID - } - }; - } - - // GitPod - if (process.env.GITPOD_WORKSPACE_ID) { - return { - source: 'gitpod', - details: { - workspace_id: process.env.GITPOD_WORKSPACE_ID.substring(0, 8) + '...' // Truncate for privacy - } - }; - } - - // CI/CD environments - if (process.env.CI) { - if (process.env.GITHUB_ACTIONS) { - return { - source: 'github-actions', - details: { - repository: process.env.GITHUB_REPOSITORY, - workflow: process.env.GITHUB_WORKFLOW - } - }; - } - if (process.env.GITLAB_CI) { - return { - source: 'gitlab-ci', - details: { - project: process.env.CI_PROJECT_NAME - } - }; - } - if (process.env.JENKINS_URL) { - return { - source: 'jenkins', - details: { - job: process.env.JOB_NAME - } - }; - } - return { - source: 'ci-cd-other', - details: { - ci_env: 'unknown' - } - }; - } - - // Docker detection - if (process.env.DOCKER_CONTAINER) { - return { - source: 'docker', - details: { - container_id: process.env.HOSTNAME?.substring(0, 8) + '...' || 'unknown' - } - }; - } - - // Check for .dockerenv file (need to use fs import) - try { - const fs = await import('fs'); - if (fs.existsSync('/.dockerenv')) { - return { - source: 'docker', - details: { - container_id: process.env.HOSTNAME?.substring(0, 8) + '...' || 'unknown' - } - }; - } - } catch (error) { - // Ignore fs errors - } - - // Default fallback - return { - source: 'unknown', - details: { - user_agent: npmConfigUserAgent || 'none', - command: npmCommand || 'none', - lifecycle: npmLifecycleEvent || 'none' - } - }; -} - -/** - * Send installation tracking to analytics - */ -async function trackInstallation(installationData) { - if (!GA_MEASUREMENT_ID || !GA_API_SECRET) { - debug('Analytics not configured, skipping tracking'); - return; - } - - try { - const uniqueUserId = await getClientId(); - log("user id", uniqueUserId) - // Prepare GA4 payload - const payload = { - client_id: uniqueUserId, - non_personalized_ads: false, - timestamp_micros: Date.now() * 1000, - events: [{ - name: 'package_installed', - params: { - timestamp: new Date().toISOString(), - platform: platform(), - installation_source: installationData.source, - installation_details: JSON.stringify(installationData.details), - package_name: '@wonderwhy-er/desktop-commander', - install_method: 'npm-lifecycle', - node_version: process.version, - npm_version: process.env.npm_version || 'unknown' - } - }] - }; - - const postData = JSON.stringify(payload); - - const options = { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'Content-Length': Buffer.byteLength(postData) - } - }; - - await new Promise((resolve, reject) => { - const req = https.request(GA_BASE_URL, options); - - const timeoutId = setTimeout(() => { - req.destroy(); - reject(new Error('Request timeout')); - }, 5000); - - req.on('error', (error) => { - clearTimeout(timeoutId); - debug(`Analytics error: ${error.message}`); - resolve(); // Don't fail installation on analytics error - }); - - req.on('response', (res) => { - clearTimeout(timeoutId); - // Consume the response data to complete the request - res.on('data', () => {}); // Ignore response data - res.on('end', () => { - log(`Installation tracked: ${installationData.source}`); - resolve(); - }); - }); - - req.write(postData); - req.end(); - }); - - } catch (error) { - debug(`Failed to track installation: ${error.message}`); - // Don't fail the installation process - } -} - -/** - * Main execution - */ -async function main() { - try { - log('Package installation detected'); - - const installationData = await detectInstallationSource(); - log(`Installation source: ${installationData.source}`); - - await trackInstallation(installationData); - - } catch (error) { - debug(`Installation tracking error: ${error.message}`); - // Don't fail the installation - } -} - -// Only run if this script is executed directly (not imported) -if (import.meta.url === `file://${process.argv[1]}`) { - main(); -} - -export { detectInstallationSource, trackInstallation }; diff --git a/wonderwhy-er-desktop-commander-0.2.9.tgz b/wonderwhy-er-desktop-commander-0.2.9.tgz deleted file mode 100644 index ac7d376a..00000000 Binary files a/wonderwhy-er-desktop-commander-0.2.9.tgz and /dev/null differ