fix(terminal): ignore errors when killing PTY on Windows

[kdrapel]

Problem
In Windows, when a task was opened, changing the working directory from the settings would not work. The terminal was not updated and the working directory was still showing the original value. You had to repeat the action a 2nd time to make the change effective.

Solution
Wrap session.pty.kill('SIGKILL') in a try/catch so killing an already-exited PTY is a no-op. This prevents the IPC call from rejecting and allows updateTaskAndNotify to complete (DB update + parent notification), fixing the working-directory two-click symptom. Typecheck passed.

Co-authored-by: Copilot

Greptile Summary

This PR fixes a Windows-specific bug in killPty where calling session.pty.kill('SIGKILL') on an already-exited PTY process throws an exception, causing the IPC call to reject and preventing updateTaskAndNotify from completing — manifesting as the "two-click" symptom when changing a task's working directory.

Changes:

• Wraps session.pty.kill('SIGKILL') in a try/catch inside killPty (pty-manager.ts) so a throw from an already-dead process is treated as a no-op.
• Session cleanup (sessions.delete + notifySessionChange) is correctly ordered before the kill call, so killPty still returns true and IPC resolves cleanly even when the catch fires.
• One minor observation: unlike the existing resizePty try/catch which records a pty.resize_failed diagnostic event, the new catch block is completely silent, which slightly reduces observability on Windows when this path is hit.

Confidence Score: 5/5

Safe to merge — minimal, well-scoped fix with correct ordering of session cleanup before the guarded kill call.

Single-function change with one added try/catch; the session is already removed from the map before kill() is called, so the return value and all callers are unaffected regardless of whether the catch fires. The pattern is consistent with the existing resizePty guard in the same file. Only concern is the silent catch vs the diagnostic-logging pattern used elsewhere, which is a style suggestion, not a correctness issue.

No files require special attention.

Important Files Changed

Filename	Overview
packages/domains/terminal/src/main/pty-manager.ts	Wraps session.pty.kill('SIGKILL') in a try/catch to silently swallow the error thrown on Windows when the PTY process has already exited; fix is correct and the session cleanup path (delete + notify) already happens before the kill call, so return true is unaffected.

Sequence Diagram

sequenceDiagram
    participant Renderer
    participant IPC as IPC Handler (handlers.ts)
    participant PM as pty-manager.ts (killPty)
    participant PTY as node-pty

    Renderer->>IPC: terminal:kill-pty (sessionId)
    IPC->>PM: killPty(sessionId)
    PM->>PM: sessions.delete(sessionId)
    PM->>PM: notifySessionChange()
    PM->>PTY: session.pty.kill('SIGKILL')
    alt PTY still alive (Linux/macOS or live Windows process)
        PTY-->>PM: (success)
    else PTY already exited (Windows)
        PTY-->>PM: throws Error
        PM->>PM: catch — treated as no-op
    end
    PM-->>IPC: return true
    IPC-->>Renderer: true (IPC resolves)

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: packages/domains/terminal/src/main/pty-manager.ts
Line: 1350-1352

Comment:
**Silent catch reduces observability**

The analogous `resizePty` try/catch (around line 1296) records a `pty.resize_failed` diagnostic event before swallowing the error, which makes it possible to correlate Windows kill failures in diagnostic logs. The new catch here is completely silent.

Consider recording a `warn`-level diagnostic event so the condition is visible in telemetry without causing the call to fail:

```suggestion
  } catch (error) {
    // Process already exited — not an error on Windows
    recordDiagnosticEvent({
      level: 'warn',
      source: 'pty',
      event: 'pty.kill_failed',
      sessionId,
      taskId: session.taskId,
      message: (error as Error).message
    })
  }
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(terminal): ignore errors when killin..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

_{(4/5) You can add custom instructions or style guidelines for the agent here!}

[greptile-apps]

Silent catch reduces observability

The analogous resizePty try/catch (around line 1296) records a pty.resize_failed diagnostic event before swallowing the error, which makes it possible to correlate Windows kill failures in diagnostic logs. The new catch here is completely silent.

Consider recording a warn-level diagnostic event so the condition is visible in telemetry without causing the call to fail:

Suggested change

	} catch {
	// Process already exited — not an error
	}
	} catch (error) {
	// Process already exited — not an error on Windows
	recordDiagnosticEvent({
	level: 'warn',
	source: 'pty',
	event: 'pty.kill_failed',
	sessionId,
	taskId: session.taskId,
	message: (error as Error).message
	})
	}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/domains/terminal/src/main/pty-manager.ts
Line: 1350-1352

Comment:
**Silent catch reduces observability**

The analogous `resizePty` try/catch (around line 1296) records a `pty.resize_failed` diagnostic event before swallowing the error, which makes it possible to correlate Windows kill failures in diagnostic logs. The new catch here is completely silent.

Consider recording a `warn`-level diagnostic event so the condition is visible in telemetry without causing the call to fail:

```suggestion
  } catch (error) {
    // Process already exited — not an error on Windows
    recordDiagnosticEvent({
      level: 'warn',
      source: 'pty',
      event: 'pty.kill_failed',
      sessionId,
      taskId: session.taskId,
      message: (error as Error).message
    })
  }
```

How can I resolve this? If you propose a fix, please make it concise.