refactor(setup): introduce Result type for clearer error handling (#218)

* feat: create error handling types and utilities

- Add Result<T, E> type for improved error handling
- Add SetupResult, BuildResult, UpdateResult type aliases
- Create comprehensive error utility functions in toolbox/system/errors.ts
- Add success/failure creators, async/sync wrappers, and helper functions
- Maintain TypeScript compilation compatibility

This establishes the foundation for removing process.exit() calls
and implementing proper error propagation throughout the CLI.

* refactor: replace process.exit() in system utilities

- Update execWithSudo, pkexec, and environment functions to return Result types
- Replace process.exit() calls with proper error returns in packages.ts
- Improve error handling in python.ts with Result wrapper
- Add error propagation instead of hard exits

Breaking change: Function signatures now return Result<T> instead of void/T.
Callers will be updated in subsequent commits.

* refactor: replace process.exit() in core setup utilities

- Update moddable.ts functions to return Result types
- Replace process.exit() calls with proper error returns in ejectfix.ts
- Wrap async operations with proper error handling
- Convert getModdableVersion, fetchRelease, downloadReleaseTools to Result pattern

Breaking change: Function signatures now return Result<T> instead of T.
Other files using these functions will be updated in subsequent commits.

* refactor: replace process.exit() in esp32 setup functions

* refactor: replace process.exit() in esp8266 setup functions

- Convert esp8266 setup functions to return Result<void> pattern
- Replace all process.exit() calls with proper error returns
- Apply same error handling pattern as ESP32 implementation
- Update function signatures for mac, linux, and windows variants
- Handle Result types from dependency installers properly

* refactor: replace process.exit() in wasm, pico, and nrf52 setup functions (partial)

- Convert wasm setup to return Result<void> pattern
- Replace process.exit() calls with proper error returns  
- Update pico setup and platform-specific variants (mac, linux)
- Update nrf52 setup and windows platform support
- Apply consistent error handling pattern across platforms
- Handle dependency installer Results properly

This is partial progress on remaining platform setup refactoring.

* refactor: replace process.exit() in linux setup functions (complete Commit 6)

- Convert linux platform setup to return Result<void> pattern
- Replace process.exit(0) with proper success return
- Handle dependency installer Results properly
- Apply consistent error handling pattern

Commit 6 now covers: wasm, pico (+ mac/linux), nrf52 (+ windows), linux.
Remaining mac.ts and windows.ts host setup files can be handled in future commits.

* refactor: update setup command to handle Result types (Commit 7)

- Update setup command to handle Result types from setup functions
- Replace process.exit() calls with appropriate returns and error handling
- Add proper error reporting with print.error() before process.exit(1)
- Handle setupEjectfix Result type properly
- Maintain existing CLI behavior while using new error handling

* refactor: remove process.exit() from remaining commands (Commit 8)

- Replace process.exit(0) with return statements in init, include, remove commands
- Maintain appropriate error reporting while allowing graceful function returns
- Remove hard exits for user-initiated cancellations and error cases
- Complete command layer refactoring to eliminate problematic process exits

* feat: add basic unit tests for error handling utilities (Commit 9)

- Add comprehensive tests for error handling utilities (19 test cases)
  - Test success, failure, wrapAsync, wrapSync functions
  - Test type guards, unwrap functions, and result chaining
  - Verify proper Result type behavior and error propagation
  
- Add unit tests for exec utilities (12 test cases)
  - Test execWithSudo with various scenarios including askpass fallback
  - Test pkexec command execution
  - Test environment sourcing functions with mocked system calls
  - Verify Result type handling across all exec functions

- All tests passing with proper TypeScript typing and mocking
- Total: 31 test cases providing solid foundation test coverage

* feat: replace outdated integration tests with comprehensive CLI tests (Commit 10)

- Replace placeholder integration tests with real CLI functionality tests
- Add comprehensive end-to-end testing covering:
  * Basic CLI functionality (version, help, command help) 
  * Error handling (unknown commands, missing arguments)
  * Command functionality (init, include, remove, setup scenarios)
  * Doctor command diagnostic checks
  * Edge cases and error conditions

- Test error scenarios end-to-end to verify new Result-based error handling
- All 13 integration tests passing with proper timeout handling
- Tests verify CLI exits gracefully with appropriate error codes and messages
- Validates that refactored error handling works correctly in real usage

Phase 1 refactoring now complete with solid test foundation!

* fix(doctor): unwrap Result from python and moddable version queries

* fix(setup/update): use correct Result type return platform modules

* fix: remove integration test errors

* refactor: resolve formatting

* refactor: resolve linting errors, ignore test files

* chore: cleanup files

* refactor: resolve CI linting

Author: HipsterBrown

.gitignore

@@ -8,5 +8,13 @@ build/
 .vscode
 !src/toolbox/build/
 .npmrc
+test-project/
 
 .env
+
+# Planning documents
+plans/
+.claude/
+.crush/
+CLAUDE.md
+CRUSH.md

__tests__/cli-integration.test.ts

@@ -1,29 +1,159 @@
-const { system, filesystem } = require('gluegun')
+import { exec } from 'node:child_process'
+import { promisify } from 'node:util'
+import { resolve } from 'node:path'
+import { filesystem } from 'gluegun'
 
-const src = filesystem.path(__dirname, '..')
+const execAsync = promisify(exec)
 
-const cli = async (cmd: string) =>
-  system.run('node ' + filesystem.path(src, 'bin', 'xs-dev') + ` ${cmd}`)
+const CLI_PATH = resolve(__dirname, '..', 'build', 'src', 'cli.js')
 
-test('outputs version', async () => {
-  const output = await cli('--version')
-  expect(output).toContain('0.0.1')
-})
+/**
+ * Execute xs-dev CLI command and return { stdout, stderr, code }
+ */
+const runCLI = async (command: string, options: { cwd?: string } = {}) => {
+  try {
+    const { stdout, stderr } = await execAsync(
+      `node "${CLI_PATH}" ${command}`,
+      {
+        cwd: options.cwd || process.cwd(),
+        timeout: 30000, // 30 second timeout
+      },
+    )
+    return { stdout: stdout.trim(), stderr: stderr.trim(), code: 0 }
+  } catch (error: any) {
+    return {
+      stdout: error.stdout?.trim() || '',
+      stderr: error.stderr?.trim() || '',
+      code: error.code || 1,
+    }
+  }
+}
 
-test('outputs help', async () => {
-  const output = await cli('--help')
-  expect(output).toContain('0.0.1')
-})
+describe('CLI Integration Tests', () => {
+  describe('Basic CLI functionality', () => {
+    it('should output version information', async () => {
+      const result = await runCLI('--version')
+
+      expect(result.code).toBe(0)
+      expect(result.stdout).toMatch(/\d+\.\d+\.\d+/)
+    }, 10000)
+
+    it('should display help information', async () => {
+      const result = await runCLI('--help')
+
+      expect(result.code).toBe(0)
+      expect(result.stdout).toContain('xs-dev')
+      expect(result.stdout).toContain('setup')
+      expect(result.stdout).toContain('build')
+      expect(result.stdout).toContain('run')
+    }, 10000)
+
+    it('should show command help', async () => {
+      const result = await runCLI('setup --help')
+
+      expect(result.code).toBe(0)
+      expect(result.stdout).toContain('setup')
+      expect(result.stdout).toContain('device')
+    }, 10000)
+  })
+
+  describe('Error handling', () => {
+    it('should handle unknown commands gracefully', async () => {
+      const result = await runCLI('nonexistent-command')
+
+      expect(result.code).not.toBe(0)
+      expect(result.stderr || result.stdout).toMatch(
+        /no command registered|unknown|not found|invalid/i,
+      )
+    }, 10000)
+
+    it('should validate missing required arguments', async () => {
+      const result = await runCLI('init') // init requires project name
+
+      expect(result.code).not.toBe(0)
+      expect(result.stderr || result.stdout).toMatch(/required|name/i)
+    }, 10000)
+  })
+
+  describe('Command functionality', () => {
+    const tempDir = resolve(__dirname, '..', 'test-project')
+
+    beforeEach(() => {
+      // Clean up any previous test artifacts
+      filesystem.remove(tempDir)
+    })
+
+    afterEach(() => {
+      // Clean up test artifacts
+      filesystem.remove(tempDir)
+    })
+
+    it('should handle init command without MODDABLE env var', async () => {
+      const result = await runCLI(`init test-project --typescript`)
+
+      expect(result.code).toBe(0)
+    }, 15000)
+
+    it('should handle include command outside of project directory', async () => {
+      const result = await runCLI('include base/timer')
+      expect(result.code).not.toBe(0)
+      expect(result.stderr || result.stdout).toMatch(
+        /manifest\.json.*not found|project directory/i,
+      )
+    }, 10000)
+
+    it('should handle remove command outside of project directory', async () => {
+      const result = await runCLI('remove timer')
+
+      expect(result.code).not.toBe(0)
+      expect(result.stderr || result.stdout).toMatch(
+        /manifest\.json.*not found|project directory/i,
+      )
+    }, 10000)
+
+    it('should handle setup command gracefully when dependencies missing', async () => {
+      const result = await runCLI('setup --device wasm')
+
+      // Should either succeed or provide helpful error messages
+      // Since we don't have Moddable SDK installed in test environment,
+      // it should fail gracefully with a helpful message
+      expect(result.code).not.toBe(0)
+      expect(result.stderr || result.stdout).toMatch(
+        /moddable|required|setup|tooling/i,
+      )
+    }, 20000)
+
+    it('should show device help information', async () => {
+      const result = await runCLI('setup --help')
+
+      // Should mention device options in help
+      expect(result.stdout || result.stderr).toMatch(
+        /device|esp32|esp8266|wasm|pico/i,
+      )
+    }, 10000)
+  })
+
+  describe('Doctor command', () => {
+    it('should run diagnostic checks', async () => {
+      const result = await runCLI('doctor')
+
+      expect(result.code).toBe(0)
+      expect(result.stdout).toMatch(/node|system|platform/i)
+    }, 15000)
+  })
 
-test('generates file', async () => {
-  const output = await cli('generate foo')
+  describe('Edge cases and error conditions', () => {
+    it('should handle invalid flags gracefully', async () => {
+      const result = await runCLI('setup --invalid-flag')
 
-  expect(output).toContain('Generated file at models/foo-model.ts')
-  const foomodel = filesystem.read('models/foo-model.ts')
+      expect(result.code).not.toBe(0)
+    }, 10000)
 
-  expect(foomodel).toContain(`module.exports = {`)
-  expect(foomodel).toContain(`name: 'foo'`)
+    it('should handle empty arguments appropriately', async () => {
+      const result = await runCLI('')
 
-  // cleanup artifact
-  filesystem.remove('models')
+      // Should show help or error message
+      expect(result.stdout || result.stderr).toMatch(/help|usage|command/i)
+    }, 10000)
+  })
 })

eslint.config.js

@@ -4,5 +4,6 @@ module.exports = [
   {
     ...loveConfig,
     files: ['src/**/*.ts'],
+    ignores: ['**/__tests__/**'],
   },
 ]

package.json

@@ -98,7 +98,10 @@
   },
   "jest": {
     "preset": "ts-jest",
-    "testEnvironment": "node"
+    "testEnvironment": "node",
+    "testPathIgnorePatterns": [
+      "<rootDir>/build/"
+    ]
   },
   "volta": {
     "node": "18.14.0"
@@ -113,7 +116,14 @@
       "ejs@<3.1.7": ">=3.1.7",
       "axios@>=0.8.1 <1.6.0": ">=1.6.0",
       "minimatch@<3.0.5": ">=3.0.5"
-    }
+    },
+    "onlyBuiltDependencies": [
+      "@serialport/bindings-cpp",
+      "esbuild",
+      "node-liblzma",
+      "sharp",
+      "usb"
+    ]
   },
   "author": "HipsterBrown <headhipster@hipsterbrown.com>",
   "auto": {

src/commands/doctor.ts

@@ -6,6 +6,7 @@ import { getModdableVersion, moddableExists } from '../toolbox/setup/moddable'
 import { sourceEnvironment } from '../toolbox/system/exec'
 import { detectPython, getPythonVersion } from '../toolbox/system/python'
 import type { Device } from '../types'
+import { unwrapOr } from '../toolbox/system/errors'
 
 const command = buildCommand({
   async func(this: LocalContext) {
@@ -66,10 +67,10 @@ const command = buildCommand({
       supportedDevices.push('nrf52')
     }
 
-    const pythonVersion = (await getPythonVersion()) ?? 'Unavailable'
+    const pythonVersion = unwrapOr(await getPythonVersion(), 'Unavailable')
     const pythonPath = system.which(detectPython() ?? '') ?? 'n/a'
 
-    const moddableVersion = (await getModdableVersion()) ?? 'Not found'
+    const moddableVersion = unwrapOr(await getModdableVersion(), 'Not found')
     const moddablePath = process.env.MODDABLE ?? 'n/a'
 
     print.info('xs-dev environment info:')
@@ -103,9 +104,9 @@ const command = buildCommand({
           : [],
         supportedDevices.includes('nrf52')
           ? [
-              'NRF52 SDK Directory',
-              String(process.env.NRF_SDK_DIR ?? process.env.NRF52_SDK_PATH),
-            ]
+            'NRF52 SDK Directory',
+            String(process.env.NRF_SDK_DIR ?? process.env.NRF52_SDK_PATH),
+          ]
           : [],
       ].filter((tuple) => tuple.length !== 0),
     )

src/commands/init.ts

@@ -39,7 +39,7 @@ const command = buildCommand({
         warning(
           `Directory called ${projectName} already exists. Please pass the --overwrite flag to replace an existing project.`,
         )
-        process.exit(0)
+        return
       }
 
       await sourceEnvironment()
@@ -81,7 +81,7 @@ const command = buildCommand({
           filesystem.copy(selectedExamplePath, projectName, { overwrite })
         } else {
           warning('Please select an example template to use.')
-          process.exit(0)
+          return
         }
       } else {
         info(`Generating Moddable project: ${projectName}`)

src/commands/remove.ts

@@ -26,7 +26,7 @@ const command = buildCommand({
 
     if (moduleName === undefined) {
       print.error('Module name is required')
-      process.exit(1)
+      return
     }
 
     print.info(`Removing "${String(moduleName)}" from manifest includes`)

src/commands/setup.ts

@@ -1,11 +1,12 @@
 import { type as platformType } from 'node:os'
 import { buildCommand } from '@stricli/core'
 import type { LocalContext } from '../cli'
-import type { Device } from '../types'
+import type { Device, SetupResult } from '../types'
 import setupEjectfix from '../toolbox/setup/ejectfix'
 import { DEVICE_ALIAS } from '../toolbox/prompt/devices'
 import { MODDABLE_REPO } from '../toolbox/setup/constants'
 import type { SetupArgs } from '../toolbox/setup/types'
+import { isFailure } from '../toolbox/system/errors'
 
 interface SetupOptions {
   device?: Device
@@ -57,7 +58,7 @@ const command = buildCommand({
         target = selectedDevice as Device
       } else {
         print.warning('Please select a target device to run')
-        process.exit(0)
+        return
       }
     }
 
@@ -66,7 +67,13 @@ const command = buildCommand({
         print.warning(`Unknown tool ${tool}`)
         process.exit(1)
       }
-      if (tool === 'ejectfix') await setupEjectfix()
+      if (tool === 'ejectfix') {
+        const result = await setupEjectfix()
+        if (isFailure(result)) {
+          print.error(result.error)
+          process.exit(1)
+        }
+      }
       return
     }
     const platformDevices: Device[] = [
@@ -81,17 +88,25 @@ const command = buildCommand({
     const { default: setup } = await import(`../toolbox/setup/${target}`)
 
     if (platformDevices.includes(target)) {
-      await setup({
+      const result = await setup({
         branch,
         release,
         sourceRepo,
         interactive:
           typeof process.env.CI !== 'undefined'
             ? process.env.CI === 'false'
             : interactive,
-      })
+      }) as SetupResult
+      if (isFailure(result)) {
+        print.error(result.error)
+        process.exit(1)
+      }
     } else {
-      await setup({ branch, release })
+      const result = await setup({ branch, release }) as SetupResult
+      if (isFailure(result)) {
+        print.error(result.error)
+        process.exit(1)
+      }
     }
   },
   parameters: {