src/main/process-manager/ProcessManager.ts (1)

233-249: Consider distinguishing genuine taskkill failures from expected ones.

The debug-level logging for errors is reasonable since taskkill returns non-zero when the process is already dead. However, genuine failures (access denied, taskkill not found on PATH, etc.) will also be logged at debug level and may go unnoticed.

Consider checking the error code/message to log genuine failures at warn level:

Optional: differentiate error types

{ if (error) { - // taskkill returns non-zero if the process is already dead, which is fine - logger.debug( - '[ProcessManager] taskkill exited with error (process may already be terminated)', - 'ProcessManager', - { sessionId, pid, error: String(error) } - ); + // taskkill exit code 128 means process not found (already dead), which is expected + const isExpectedError = error.message?.includes('not found') || + (error as NodeJS.ErrnoException).code === 'ENOENT'; + const logFn = isExpectedError ? logger.debug : logger.warn; + logFn( + '[ProcessManager] taskkill exited with error', + 'ProcessManager', + { sessionId, pid, error: String(error), expected: isExpectedError } + ); } }); }"> private killWindowsProcessTree(pid: number, sessionId: string): void {
logger.info(
'[ProcessManager] Using taskkill to terminate process tree on Windows',
'ProcessManager',
{ sessionId, pid }
);
execFile('taskkill', ['/pid', String(pid), '/t', '/f'], (error) => {
if (error) {
- // taskkill returns non-zero if the process is already dead, which is fine
- logger.debug(
- '[ProcessManager] taskkill exited with error (process may already be terminated)',
- 'ProcessManager',
- { sessionId, pid, error: String(error) }
- );
+ // taskkill exit code 128 means process not found (already dead), which is expected
+ const isExpectedError = error.message?.includes('not found') ||
+ (error as NodeJS.ErrnoException).code === 'ENOENT';
+ const logFn = isExpectedError ? logger.debug : logger.warn;
+ logFn(
+ '[ProcessManager] taskkill exited with error',
+ 'ProcessManager',
+ { sessionId, pid, error: String(error), expected: isExpectedError }
+ );
}
});
}

Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/process-manager/ProcessManager.ts` around lines 233 - 249, In
killWindowsProcessTree, the execFile callback currently logs all taskkill errors
at debug level; change it to inspect the Error object returned by execFile (in
the callback inside killWindowsProcessTree) and classify expected "process
already terminated" responses versus real failures: if error.message or
error.code indicates benign conditions (e.g., contains phrases like "not
running", "no such process", "not found", or exit/status codes known to mean
"already terminated") keep logger.debug, but if error indicates genuine failures
(e.g., ENOENT/taskkill not on PATH, "Access is denied", other non-benign exit
codes or messages) log with logger.warn and include sessionId, pid and the full
error details; keep the existing debug branch for the benign case.