Fix for reding finished process outputs (#210)

* After switch to start process in some situations reading process output after it finished stopped working. Added tests and fixed issue

* Few more improvements and fixes

Author: wonderwhy-er

src/terminal-manager.ts

@@ -164,7 +164,7 @@ export class TerminalManager {
           // Store completed session before removing active session
           this.completedSessions.set(childProcess.pid, {
             pid: childProcess.pid,
-            output: output + session.lastOutput, // Combine all output
+            output: output, // Use only the main output variable
             exitCode: code,
             startTime: session.startTime,
             endTime: new Date()
@@ -199,9 +199,15 @@ export class TerminalManager {
     // Then check completed sessions
     const completedSession = this.completedSessions.get(pid);
     if (completedSession) {
-      // Format completion message with exit code and runtime
+      // Format with output first, then completion info
       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}`;
+      const output = completedSession.output.trim();
+      
+      if (output) {
+        return `${output}\n\nProcess completed with exit code ${completedSession.exitCode}\nRuntime: ${runtime}s`;
+      } else {
+        return `Process completed with exit code ${completedSession.exitCode}\nRuntime: ${runtime}s\n(No output produced)`;
+      }
     }
 
     return null;

src/tools/improved-process-tools.ts

@@ -110,8 +110,20 @@ export async function readProcessOutput(args: unknown): Promise<ServerResult> {
 
   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 active session found for PID ${pid}` }],
+      content: [{ type: "text", text: `No session found for PID ${pid}` }],
       isError: true,
     };
   }

test/test-read-completed-process.js

@@ -0,0 +1,98 @@
+import assert from 'assert';
+import { startProcess, readProcessOutput } from '../dist/tools/improved-process-tools.js';
+
+/**
+ * Proper test for read_process_output on completed processes
+ * 
+ * This test should:
+ * - FAIL when the bug exists (current behavior)  
+ * - PASS when the bug is fixed (desired behavior)
+ */
+async function testReadCompletedProcessOutput() {
+  console.log('Testing read_process_output on completed process...');
+  
+  // Start echo command with delay, but timeout before echo happens
+  const startResult = await startProcess({
+    // Cross-platform delay + output using Node
+    command: 'node -e "setTimeout(() => console.log(\'SUCCESS MESSAGE\'), 1000)"',
+    timeout_ms: 500  // Returns before the output happens
+  });
+  
+  // Extract PID
+  const pidMatch = startResult.content[0].text.match(/Process started with PID (\d+)/);
+  assert(pidMatch, 'Should get PID from start_process');
+  const pid = parseInt(pidMatch[1]);
+  
+  // Wait for the actual command to complete
+  await new Promise(resolve => setTimeout(resolve, 2000));
+  
+  // Try to read the output - this should work when fixed
+  const readResult = await readProcessOutput({ pid, timeout_ms: 1000 });
+  
+  // ASSERT: Should be able to read from completed process
+  assert(!readResult.isError, 
+    'Should be able to read from completed process without error');
+    
+  // ASSERT: Should contain the echo output
+  assert(readResult.content[0].text.includes('SUCCESS MESSAGE'), 
+    'Should contain the echo output from completed process');
+    
+  console.log('✅ Successfully read from completed process');
+  console.log('✅ Retrieved echo output:', readResult.content[0].text);
+}
+
+/**
+ * Test immediate completion scenario
+ */
+async function testImmediateCompletion() {
+  console.log('Testing immediate completion...');
+  
+  const startResult = await startProcess({
+    command: 'node -e "console.log(\'IMMEDIATE OUTPUT\')"',
+    timeout_ms: 2000
+  });
+  
+  // Extract PID
+  const pidMatch = startResult.content[0].text.match(/Process started with PID (\d+)/);
+  assert(pidMatch, 'Should get PID from start_process');
+  const pid = parseInt(pidMatch[1]);
+  
+  // Small delay to ensure process completed
+  await new Promise(resolve => setTimeout(resolve, 100));
+  
+  // Should be able to read from immediately completed process
+  const readResult = await readProcessOutput({ pid, timeout_ms: 1000 });
+  
+  assert(!readResult.isError, 
+    'Should be able to read from immediately completed process');
+    
+  assert(readResult.content[0].text.includes('IMMEDIATE OUTPUT'), 
+    'Should contain immediate output from completed process');
+    
+  console.log('✅ Successfully read from immediately completed process');
+}
+
+// Run tests
+async function runTests() {
+  try {
+    await testReadCompletedProcessOutput();
+    await testImmediateCompletion();
+    console.log('\n🎉 All tests passed - read_process_output works on completed processes!');
+    return true;
+  } catch (error) {
+    console.log('\n❌ Test failed:', error.message);
+    console.log('\n💡 This indicates the bug still exists:');
+    console.log('   read_process_output cannot read from completed processes');
+    console.log('   Expected behavior: Should return completion info and final output');
+    return false;
+  }
+}
+
+runTests()
+  .then(success => {
+    process.exit(success ? 0 : 1);
+  })
+  .catch(error => {
+    console.error('Test error:', error);
+    process.exit(1);
+  });