Windows: Refactor Shell Scripts to Node.js for Cross-Platform Compatibility (#784)

packages/cli/src/ui/hooks/shellCommandProcessor.test.ts

@@ -44,8 +44,10 @@ vi.mock('path', () => ({
 vi.mock('os', () => ({
   default: {
     tmpdir: vi.fn(() => '/tmp'),
+    platform: vi.fn(() => 'linux'),
   },
   tmpdir: vi.fn(() => '/tmp'),
+  platform: vi.fn(() => 'linux'),
 }));
 
 // Configure the fs mock to use new vi.fn() instances created within the factory

packages/cli/src/ui/hooks/shellCommandProcessor.ts

@@ -43,13 +43,23 @@ export const useShellCommandProcessor = (
         return false;
       }
 
-      // wrap command to write pwd to temporary file
-      let commandToExecute = rawQuery.trim();
-      const pwdFileName = `shell_pwd_${crypto.randomBytes(6).toString('hex')}.tmp`;
-      const pwdFilePath = path.join(os.tmpdir(), pwdFileName);
-      if (!commandToExecute.endsWith('&')) commandToExecute += ';';
-      // note here we could also restore a previous pwd with `cd {cwd}; { ... }`
-      commandToExecute = `{ ${commandToExecute} }; __code=$?; pwd >${pwdFilePath}; exit $__code`;
+      const isWindows = os.platform() === 'win32';
+      let commandToExecute: string;
+      let pwdFilePath: string | undefined;
+
+      if (isWindows) {
+        commandToExecute = rawQuery;
+      } else {
+        // wrap command to write pwd to temporary file
+        let command = rawQuery.trim();
+        const pwdFileName = `shell_pwd_${crypto
+          .randomBytes(6)
+          .toString('hex')}.tmp`;
+        pwdFilePath = path.join(os.tmpdir(), pwdFileName);
+        if (!command.endsWith('&')) command += ';';
+        // note here we could also restore a previous pwd with `cd {cwd}; { ... }`
+        commandToExecute = `{ ${command} }; __code=$?; pwd >${pwdFilePath}; exit $__code`;
+      }
 
       const userMessageTimestamp = Date.now();
       addItemToHistory(
@@ -101,7 +111,7 @@ export const useShellCommandProcessor = (
                   userMessageTimestamp,
                 );
               }
-              if (fs.existsSync(pwdFilePath)) {
+              if (pwdFilePath && fs.existsSync(pwdFilePath)) {
                 const pwd = fs.readFileSync(pwdFilePath, 'utf8').trim();
                 if (pwd !== targetDir) {
                   addItemToHistory(
@@ -118,11 +128,16 @@ export const useShellCommandProcessor = (
             },
           );
         } else {
-          const child = spawn('bash', ['-c', commandToExecute], {
-            cwd: targetDir,
-            stdio: ['ignore', 'pipe', 'pipe'],
-            detached: true, // Important for process group killing
-          });
+          const child = isWindows
+            ? spawn('cmd.exe', ['/c', commandToExecute], {
+                cwd: targetDir,
+                stdio: ['ignore', 'pipe', 'pipe'],
+              })
+            : spawn('bash', ['-c', commandToExecute], {
+                cwd: targetDir,
+                stdio: ['ignore', 'pipe', 'pipe'],
+                detached: true, // Important for process group killing
+              });
 
           let exited = false;
           let output = '';
@@ -155,24 +170,29 @@ export const useShellCommandProcessor = (
               onDebugMessage(
                 `Aborting shell command (PID: ${child.pid}) due to signal.`,
               );
-              try {
-                // attempt to SIGTERM process group (negative PID)
-                // fall back to SIGKILL (to group) after 200ms
-                process.kill(-child.pid, 'SIGTERM');
-                await new Promise((resolve) => setTimeout(resolve, 200));
-                if (child.pid && !exited) {
-                  process.kill(-child.pid, 'SIGKILL');
-                }
-              } catch (_e) {
-                // if group kill fails, fall back to killing just the main process
+              if (os.platform() === 'win32') {
+                // For Windows, use taskkill to kill the process tree
+                spawn('taskkill', ['/pid', child.pid.toString(), '/f', '/t']);
+              } else {
                 try {
-                  if (child.pid) {
-                    child.kill('SIGKILL');
+                  // attempt to SIGTERM process group (negative PID)
+                  // fall back to SIGKILL (to group) after 200ms
+                  process.kill(-child.pid, 'SIGTERM');
+                  await new Promise((resolve) => setTimeout(resolve, 200));
+                  if (child.pid && !exited) {
+                    process.kill(-child.pid, 'SIGKILL');
                   }
                 } catch (_e) {
-                  console.error(
-                    `failed to kill shell process ${child.pid}: ${_e}`,
-                  );
+                  // if group kill fails, fall back to killing just the main process
+                  try {
+                    if (child.pid) {
+                      child.kill('SIGKILL');
+                    }
+                  } catch (_e) {
+                    console.error(
+                      `failed to kill shell process ${child.pid}: ${_e}`,
+                    );
+                  }
                 }
               }
             }
@@ -208,7 +228,7 @@ export const useShellCommandProcessor = (
                 userMessageTimestamp,
               );
             }
-            if (fs.existsSync(pwdFilePath)) {
+            if (pwdFilePath && fs.existsSync(pwdFilePath)) {
               const pwd = fs.readFileSync(pwdFilePath, 'utf8').trim();
               if (pwd !== targetDir) {
                 addItemToHistory(

packages/cli/src/ui/hooks/useLoadingIndicator.test.ts

@@ -5,7 +5,7 @@
  */
 
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { act, renderHook } from '@testing-library/react';
+import { renderHook, act } from '@testing-library/react';
 import { useLoadingIndicator } from './useLoadingIndicator.js';
 import { StreamingState } from '../types.js';
 import {
@@ -32,7 +32,7 @@ describe('useLoadingIndicator', () => {
     expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]);
   });
 
-  it('should reflect values when Responding', () => {
+  it('should reflect values when Responding', async () => {
     const { result } = renderHook(() =>
       useLoadingIndicator(StreamingState.Responding),
     );
@@ -42,29 +42,33 @@ describe('useLoadingIndicator', () => {
     expect(WITTY_LOADING_PHRASES).toContain(
       result.current.currentLoadingPhrase,
     );
-    const _initialPhrase = result.current.currentLoadingPhrase;
+    const initialPhrase = result.current.currentLoadingPhrase;
 
-    act(() => {
-      vi.advanceTimersByTime(PHRASE_CHANGE_INTERVAL_MS);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(PHRASE_CHANGE_INTERVAL_MS);
     });
+
     // Phrase should cycle if PHRASE_CHANGE_INTERVAL_MS has passed
+    expect(result.current.currentLoadingPhrase).not.toBe(initialPhrase);
     expect(WITTY_LOADING_PHRASES).toContain(
       result.current.currentLoadingPhrase,
     );
   });
 
-  it('should show waiting phrase and retain elapsedTime when WaitingForConfirmation', () => {
+  it('should show waiting phrase and retain elapsedTime when WaitingForConfirmation', async () => {
     const { result, rerender } = renderHook(
       ({ streamingState }) => useLoadingIndicator(streamingState),
       { initialProps: { streamingState: StreamingState.Responding } },
     );
 
-    act(() => {
-      vi.advanceTimersByTime(60000);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(60000);
     });
     expect(result.current.elapsedTime).toBe(60);
 
-    rerender({ streamingState: StreamingState.WaitingForConfirmation });
+    act(() => {
+      rerender({ streamingState: StreamingState.WaitingForConfirmation });
+    });
 
     expect(result.current.currentLoadingPhrase).toBe(
       'Waiting for user confirmation...',
@@ -72,60 +76,66 @@ describe('useLoadingIndicator', () => {
     expect(result.current.elapsedTime).toBe(60); // Elapsed time should be retained
 
     // Timer should not advance further
-    act(() => {
-      vi.advanceTimersByTime(2000);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(2000);
     });
     expect(result.current.elapsedTime).toBe(60);
   });
 
-  it('should reset elapsedTime and use a witty phrase when transitioning from WaitingForConfirmation to Responding', () => {
+  it('should reset elapsedTime and use a witty phrase when transitioning from WaitingForConfirmation to Responding', async () => {
     const { result, rerender } = renderHook(
       ({ streamingState }) => useLoadingIndicator(streamingState),
       { initialProps: { streamingState: StreamingState.Responding } },
     );
 
-    act(() => {
-      vi.advanceTimersByTime(5000); // 5s
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(5000); // 5s
     });
     expect(result.current.elapsedTime).toBe(5);
 
-    rerender({ streamingState: StreamingState.WaitingForConfirmation });
+    act(() => {
+      rerender({ streamingState: StreamingState.WaitingForConfirmation });
+    });
     expect(result.current.elapsedTime).toBe(5);
     expect(result.current.currentLoadingPhrase).toBe(
       'Waiting for user confirmation...',
     );
 
-    rerender({ streamingState: StreamingState.Responding });
+    act(() => {
+      rerender({ streamingState: StreamingState.Responding });
+    });
     expect(result.current.elapsedTime).toBe(0); // Should reset
     expect(WITTY_LOADING_PHRASES).toContain(
       result.current.currentLoadingPhrase,
     );
 
-    act(() => {
-      vi.advanceTimersByTime(1000);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(1000);
     });
     expect(result.current.elapsedTime).toBe(1);
   });
 
-  it('should reset timer and phrase when streamingState changes from Responding to Idle', () => {
+  it('should reset timer and phrase when streamingState changes from Responding to Idle', async () => {
     const { result, rerender } = renderHook(
       ({ streamingState }) => useLoadingIndicator(streamingState),
       { initialProps: { streamingState: StreamingState.Responding } },
     );
 
-    act(() => {
-      vi.advanceTimersByTime(10000); // 10s
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(10000); // 10s
     });
     expect(result.current.elapsedTime).toBe(10);
 
-    rerender({ streamingState: StreamingState.Idle });
+    act(() => {
+      rerender({ streamingState: StreamingState.Idle });
+    });
 
     expect(result.current.elapsedTime).toBe(0);
     expect(result.current.currentLoadingPhrase).toBe(WITTY_LOADING_PHRASES[0]);
 
     // Timer should not advance
-    act(() => {
-      vi.advanceTimersByTime(2000);
+    await act(async () => {
+      await vi.advanceTimersByTimeAsync(2000);
     });
     expect(result.current.elapsedTime).toBe(0);
   });