Explorar el Código

refactor: move interpretExitCode from TerminalManager to TerminalProcess

This commit moves the interpretExitCode method from TerminalManager to
TerminalProcess class, as part of the terminal refactoring effort. The
method is responsible for translating exit codes into detailed results,
including signal information.

Changes include:
- Moved interpretExitCode method to TerminalProcess class
- Updated imports in TerminalManager and Cline to reference
  ExitCodeDetails from TerminalProcess
- Added findTerminalIdByVscodeTerminal helper method in TerminalManager
- Added comprehensive unit tests for interpretExitCode in
  TerminalProcess
- Tests cover undefined exit codes, normal exit codes (0-127), and
  signal exit codes (128+)

This change improves code organization by placing the exit code
interpretation logic closer to where it's primarily used, in the
TerminalProcess class.

Signed-off-by: Eric Wheeler <[email protected]>
Eric Wheeler hace 1 año
padre
commit
daf36f38ea

+ 2 - 1
src/core/Cline.ts

@@ -27,7 +27,8 @@ import {
 	everyLineHasLineNumbers,
 	everyLineHasLineNumbers,
 	truncateOutput,
 	truncateOutput,
 } from "../integrations/misc/extract-text"
 } from "../integrations/misc/extract-text"
-import { TerminalManager, ExitCodeDetails } from "../integrations/terminal/TerminalManager"
+import { TerminalManager } from "../integrations/terminal/TerminalManager"
+import { ExitCodeDetails } from "../integrations/terminal/TerminalProcess"
 import { UrlContentFetcher } from "../services/browser/UrlContentFetcher"
 import { UrlContentFetcher } from "../services/browser/UrlContentFetcher"
 import { listFiles } from "../services/glob/list-files"
 import { listFiles } from "../services/glob/list-files"
 import { regexSearchFiles } from "../services/ripgrep"
 import { regexSearchFiles } from "../services/ripgrep"

+ 20 - 103
src/integrations/terminal/TerminalManager.ts

@@ -1,7 +1,7 @@
 import pWaitFor from "p-wait-for"
 import pWaitFor from "p-wait-for"
 import * as vscode from "vscode"
 import * as vscode from "vscode"
 import { arePathsEqual } from "../../utils/path"
 import { arePathsEqual } from "../../utils/path"
-import { mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess"
+import { ExitCodeDetails, mergePromise, TerminalProcess, TerminalProcessResultPromise } from "./TerminalProcess"
 import { Terminal } from "./Terminal"
 import { Terminal } from "./Terminal"
 import { TerminalRegistry } from "./TerminalRegistry"
 import { TerminalRegistry } from "./TerminalRegistry"
 
 
@@ -95,112 +95,11 @@ declare module "vscode" {
 	}
 	}
 }
 }
 
 
-export interface ExitCodeDetails {
-	exitCode: number | undefined
-	signal?: number | undefined
-	signalName?: string
-	coreDumpPossible?: boolean
-}
-
 export class TerminalManager {
 export class TerminalManager {
 	private terminalIds: Set<number> = new Set()
 	private terminalIds: Set<number> = new Set()
 	private processes: Map<number, TerminalProcess> = new Map()
 	private processes: Map<number, TerminalProcess> = new Map()
 	private disposables: vscode.Disposable[] = []
 	private disposables: vscode.Disposable[] = []
 
 
-	private interpretExitCode(exitCode: number | undefined): ExitCodeDetails {
-		if (exitCode === undefined) {
-			return { exitCode }
-		}
-
-		if (exitCode <= 128) {
-			return { exitCode }
-		}
-
-		const signal = exitCode - 128
-		const signals: Record<number, string> = {
-			// Standard signals
-			1: "SIGHUP",
-			2: "SIGINT",
-			3: "SIGQUIT",
-			4: "SIGILL",
-			5: "SIGTRAP",
-			6: "SIGABRT",
-			7: "SIGBUS",
-			8: "SIGFPE",
-			9: "SIGKILL",
-			10: "SIGUSR1",
-			11: "SIGSEGV",
-			12: "SIGUSR2",
-			13: "SIGPIPE",
-			14: "SIGALRM",
-			15: "SIGTERM",
-			16: "SIGSTKFLT",
-			17: "SIGCHLD",
-			18: "SIGCONT",
-			19: "SIGSTOP",
-			20: "SIGTSTP",
-			21: "SIGTTIN",
-			22: "SIGTTOU",
-			23: "SIGURG",
-			24: "SIGXCPU",
-			25: "SIGXFSZ",
-			26: "SIGVTALRM",
-			27: "SIGPROF",
-			28: "SIGWINCH",
-			29: "SIGIO",
-			30: "SIGPWR",
-			31: "SIGSYS",
-
-			// Real-time signals base
-			34: "SIGRTMIN",
-
-			// SIGRTMIN+n signals
-			35: "SIGRTMIN+1",
-			36: "SIGRTMIN+2",
-			37: "SIGRTMIN+3",
-			38: "SIGRTMIN+4",
-			39: "SIGRTMIN+5",
-			40: "SIGRTMIN+6",
-			41: "SIGRTMIN+7",
-			42: "SIGRTMIN+8",
-			43: "SIGRTMIN+9",
-			44: "SIGRTMIN+10",
-			45: "SIGRTMIN+11",
-			46: "SIGRTMIN+12",
-			47: "SIGRTMIN+13",
-			48: "SIGRTMIN+14",
-			49: "SIGRTMIN+15",
-
-			// SIGRTMAX-n signals
-			50: "SIGRTMAX-14",
-			51: "SIGRTMAX-13",
-			52: "SIGRTMAX-12",
-			53: "SIGRTMAX-11",
-			54: "SIGRTMAX-10",
-			55: "SIGRTMAX-9",
-			56: "SIGRTMAX-8",
-			57: "SIGRTMAX-7",
-			58: "SIGRTMAX-6",
-			59: "SIGRTMAX-5",
-			60: "SIGRTMAX-4",
-			61: "SIGRTMAX-3",
-			62: "SIGRTMAX-2",
-			63: "SIGRTMAX-1",
-			64: "SIGRTMAX",
-		}
-
-		// These signals may produce core dumps:
-		//   SIGQUIT, SIGILL, SIGABRT, SIGBUS, SIGFPE, SIGSEGV
-		const coreDumpPossible = new Set([3, 4, 6, 7, 8, 11])
-
-		return {
-			exitCode,
-			signal,
-			signalName: signals[signal] || `Unknown Signal (${signal})`,
-			coreDumpPossible: coreDumpPossible.has(signal),
-		}
-	}
-
 	constructor() {
 	constructor() {
 		let startDisposable: vscode.Disposable | undefined
 		let startDisposable: vscode.Disposable | undefined
 		let endDisposable: vscode.Disposable | undefined
 		let endDisposable: vscode.Disposable | undefined
@@ -231,7 +130,10 @@ export class TerminalManager {
 
 
 			// onDidEndTerminalShellExecution
 			// onDidEndTerminalShellExecution
 			endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => {
 			endDisposable = (vscode.window as vscode.Window).onDidEndTerminalShellExecution?.(async (e) => {
-				const exitDetails = this.interpretExitCode(e?.exitCode)
+				// Find the terminal ID by the VSCode terminal instance
+				const terminalId = this.findTerminalIdByVscodeTerminal(e.terminal)
+				const process = terminalId !== undefined ? this.processes.get(terminalId) : undefined
+				const exitDetails = process ? process.interpretExitCode(e?.exitCode) : { exitCode: e?.exitCode }
 				console.info("[TerminalManager] Shell execution ended:", {
 				console.info("[TerminalManager] Shell execution ended:", {
 					...exitDetails,
 					...exitDetails,
 				})
 				})
@@ -356,6 +258,21 @@ export class TerminalManager {
 		return process ? process.getUnretrievedOutput() : ""
 		return process ? process.getUnretrievedOutput() : ""
 	}
 	}
 
 
+	/**
+	 * Finds the terminal ID by the VSCode terminal instance
+	 * @param terminal The VSCode terminal instance
+	 * @returns The terminal ID or undefined if not found
+	 */
+	private findTerminalIdByVscodeTerminal(terminal: vscode.Terminal): number | undefined {
+		for (const id of this.terminalIds) {
+			const info = TerminalRegistry.getTerminal(id)
+			if (info && info.terminal === terminal) {
+				return id
+			}
+		}
+		return undefined
+	}
+
 	isProcessHot(terminalId: number): boolean {
 	isProcessHot(terminalId: number): boolean {
 		const process = this.processes.get(terminalId)
 		const process = this.processes.get(terminalId)
 		return process ? process.isHot : false
 		return process ? process.isHot : false

+ 100 - 1
src/integrations/terminal/TerminalProcess.ts

@@ -3,7 +3,12 @@ import stripAnsi from "strip-ansi"
 import * as vscode from "vscode"
 import * as vscode from "vscode"
 import { inspect } from "util"
 import { inspect } from "util"
 
 
-import { ExitCodeDetails } from "./TerminalManager"
+export interface ExitCodeDetails {
+	exitCode: number | undefined
+	signal?: number | undefined
+	signalName?: string
+	coreDumpPossible?: boolean
+}
 import { Terminal } from "./Terminal"
 import { Terminal } from "./Terminal"
 import { TerminalRegistry } from "./TerminalRegistry"
 import { TerminalRegistry } from "./TerminalRegistry"
 
 
@@ -34,6 +39,100 @@ export class TerminalProcess extends EventEmitter<TerminalProcessEvents> {
 	private fullOutput: string = ""
 	private fullOutput: string = ""
 	private lastRetrievedIndex: number = 0
 	private lastRetrievedIndex: number = 0
 	isHot: boolean = false
 	isHot: boolean = false
+
+	interpretExitCode(exitCode: number | undefined): ExitCodeDetails {
+		if (exitCode === undefined) {
+			return { exitCode }
+		}
+
+		if (exitCode <= 128) {
+			return { exitCode }
+		}
+
+		const signal = exitCode - 128
+		const signals: Record<number, string> = {
+			// Standard signals
+			1: "SIGHUP",
+			2: "SIGINT",
+			3: "SIGQUIT",
+			4: "SIGILL",
+			5: "SIGTRAP",
+			6: "SIGABRT",
+			7: "SIGBUS",
+			8: "SIGFPE",
+			9: "SIGKILL",
+			10: "SIGUSR1",
+			11: "SIGSEGV",
+			12: "SIGUSR2",
+			13: "SIGPIPE",
+			14: "SIGALRM",
+			15: "SIGTERM",
+			16: "SIGSTKFLT",
+			17: "SIGCHLD",
+			18: "SIGCONT",
+			19: "SIGSTOP",
+			20: "SIGTSTP",
+			21: "SIGTTIN",
+			22: "SIGTTOU",
+			23: "SIGURG",
+			24: "SIGXCPU",
+			25: "SIGXFSZ",
+			26: "SIGVTALRM",
+			27: "SIGPROF",
+			28: "SIGWINCH",
+			29: "SIGIO",
+			30: "SIGPWR",
+			31: "SIGSYS",
+
+			// Real-time signals base
+			34: "SIGRTMIN",
+
+			// SIGRTMIN+n signals
+			35: "SIGRTMIN+1",
+			36: "SIGRTMIN+2",
+			37: "SIGRTMIN+3",
+			38: "SIGRTMIN+4",
+			39: "SIGRTMIN+5",
+			40: "SIGRTMIN+6",
+			41: "SIGRTMIN+7",
+			42: "SIGRTMIN+8",
+			43: "SIGRTMIN+9",
+			44: "SIGRTMIN+10",
+			45: "SIGRTMIN+11",
+			46: "SIGRTMIN+12",
+			47: "SIGRTMIN+13",
+			48: "SIGRTMIN+14",
+			49: "SIGRTMIN+15",
+
+			// SIGRTMAX-n signals
+			50: "SIGRTMAX-14",
+			51: "SIGRTMAX-13",
+			52: "SIGRTMAX-12",
+			53: "SIGRTMAX-11",
+			54: "SIGRTMAX-10",
+			55: "SIGRTMAX-9",
+			56: "SIGRTMAX-8",
+			57: "SIGRTMAX-7",
+			58: "SIGRTMAX-6",
+			59: "SIGRTMAX-5",
+			60: "SIGRTMAX-4",
+			61: "SIGRTMAX-3",
+			62: "SIGRTMAX-2",
+			63: "SIGRTMAX-1",
+			64: "SIGRTMAX",
+		}
+
+		// These signals may produce core dumps:
+		//   SIGQUIT, SIGILL, SIGABRT, SIGBUS, SIGFPE, SIGSEGV
+		const coreDumpPossible = new Set([3, 4, 6, 7, 8, 11])
+
+		return {
+			exitCode,
+			signal,
+			signalName: signals[signal] || `Unknown Signal (${signal})`,
+			coreDumpPossible: coreDumpPossible.has(signal),
+		}
+	}
 	private hotTimer: NodeJS.Timeout | null = null
 	private hotTimer: NodeJS.Timeout | null = null
 
 
 	async run(terminal: vscode.Terminal, command: string) {
 	async run(terminal: vscode.Terminal, command: string) {

+ 159 - 0
src/integrations/terminal/__tests__/TerminalProcessInterpretExitCode.test.ts

@@ -0,0 +1,159 @@
+import { TerminalProcess } from "../TerminalProcess"
+import { execSync } from "child_process"
+
+describe("TerminalProcess.interpretExitCode", () => {
+	let terminalProcess: TerminalProcess
+
+	beforeEach(() => {
+		terminalProcess = new TerminalProcess()
+	})
+
+	it("should handle undefined exit code", () => {
+		const result = terminalProcess.interpretExitCode(undefined)
+		expect(result).toEqual({ exitCode: undefined })
+	})
+
+	it("should handle normal exit codes (0-127)", () => {
+		// Test success exit code (0)
+		let result = terminalProcess.interpretExitCode(0)
+		expect(result).toEqual({ exitCode: 0 })
+
+		// Test error exit code (1)
+		result = terminalProcess.interpretExitCode(1)
+		expect(result).toEqual({ exitCode: 1 })
+
+		// Test arbitrary exit code within normal range
+		result = terminalProcess.interpretExitCode(42)
+		expect(result).toEqual({ exitCode: 42 })
+
+		// Test boundary exit code
+		result = terminalProcess.interpretExitCode(127)
+		expect(result).toEqual({ exitCode: 127 })
+	})
+
+	it("should handle signal exit codes (128+)", () => {
+		// Test SIGINT (Ctrl+C) - 128 + 2 = 130
+		const result = terminalProcess.interpretExitCode(130)
+		expect(result).toEqual({
+			exitCode: 130,
+			signal: 2,
+			signalName: "SIGINT",
+			coreDumpPossible: false,
+		})
+
+		// Test SIGTERM - 128 + 15 = 143
+		const resultTerm = terminalProcess.interpretExitCode(143)
+		expect(resultTerm).toEqual({
+			exitCode: 143,
+			signal: 15,
+			signalName: "SIGTERM",
+			coreDumpPossible: false,
+		})
+
+		// Test SIGSEGV (segmentation fault) - 128 + 11 = 139
+		const resultSegv = terminalProcess.interpretExitCode(139)
+		expect(resultSegv).toEqual({
+			exitCode: 139,
+			signal: 11,
+			signalName: "SIGSEGV",
+			coreDumpPossible: true,
+		})
+	})
+
+	it("should identify signals that can produce core dumps", () => {
+		// Core dump possible signals: SIGQUIT(3), SIGILL(4), SIGABRT(6), SIGBUS(7), SIGFPE(8), SIGSEGV(11)
+		const coreDumpSignals = [3, 4, 6, 7, 8, 11]
+
+		for (const signal of coreDumpSignals) {
+			const exitCode = 128 + signal
+			const result = terminalProcess.interpretExitCode(exitCode)
+			expect(result.coreDumpPossible).toBe(true)
+		}
+
+		// Test a non-core-dump signal
+		const nonCoreDumpResult = terminalProcess.interpretExitCode(128 + 1) // SIGHUP
+		expect(nonCoreDumpResult.coreDumpPossible).toBe(false)
+	})
+
+	it("should handle unknown signals", () => {
+		// Test an exit code for a signal that's not in our mapping
+		const result = terminalProcess.interpretExitCode(128 + 99)
+		expect(result).toEqual({
+			exitCode: 128 + 99,
+			signal: 99,
+			signalName: "Unknown Signal (99)",
+			coreDumpPossible: false,
+		})
+	})
+})
+
+describe("TerminalProcess.interpretExitCode with real commands", () => {
+	let terminalProcess: TerminalProcess
+
+	beforeEach(() => {
+		terminalProcess = new TerminalProcess()
+	})
+
+	it("should correctly interpret exit code 0 from successful command", () => {
+		try {
+			// Run a command that should succeed
+			execSync("echo test", { stdio: "ignore" })
+			// If we get here, the command succeeded with exit code 0
+			const result = terminalProcess.interpretExitCode(0)
+			expect(result).toEqual({ exitCode: 0 })
+		} catch (error: any) {
+			// This should not happen for a successful command
+			fail("Command should have succeeded: " + error.message)
+		}
+	})
+
+	it("should correctly interpret exit code 1 from failed command", () => {
+		try {
+			// Run a command that should fail with exit code 1 or 2
+			execSync("ls /nonexistent_directory", { stdio: "ignore" })
+			fail("Command should have failed")
+		} catch (error: any) {
+			// Verify the exit code is what we expect (can be 1 or 2 depending on the system)
+			expect(error.status).toBeGreaterThan(0)
+			expect(error.status).toBeLessThan(128) // Not a signal
+			const result = terminalProcess.interpretExitCode(error.status)
+			expect(result).toEqual({ exitCode: error.status })
+		}
+	})
+
+	it("should correctly interpret exit code from command with custom exit code", () => {
+		try {
+			// Run a command that exits with a specific code
+			execSync("exit 42", { stdio: "ignore" })
+			fail("Command should have exited with code 42")
+		} catch (error: any) {
+			expect(error.status).toBe(42)
+			const result = terminalProcess.interpretExitCode(error.status)
+			expect(result).toEqual({ exitCode: 42 })
+		}
+	})
+
+	// Test signal interpretation directly without relying on actual process termination
+	it("should correctly interpret signal termination codes", () => {
+		// Test SIGTERM (signal 15)
+		const sigtermExitCode = 128 + 15
+		const sigtermResult = terminalProcess.interpretExitCode(sigtermExitCode)
+		expect(sigtermResult.signal).toBe(15)
+		expect(sigtermResult.signalName).toBe("SIGTERM")
+		expect(sigtermResult.coreDumpPossible).toBe(false)
+
+		// Test SIGSEGV (signal 11)
+		const sigsegvExitCode = 128 + 11
+		const sigsegvResult = terminalProcess.interpretExitCode(sigsegvExitCode)
+		expect(sigsegvResult.signal).toBe(11)
+		expect(sigsegvResult.signalName).toBe("SIGSEGV")
+		expect(sigsegvResult.coreDumpPossible).toBe(true)
+
+		// Test SIGINT (signal 2)
+		const sigintExitCode = 128 + 2
+		const sigintResult = terminalProcess.interpretExitCode(sigintExitCode)
+		expect(sigintResult.signal).toBe(2)
+		expect(sigintResult.signalName).toBe("SIGINT")
+		expect(sigintResult.coreDumpPossible).toBe(false)
+	})
+})