| author |  Dara Adedeji <76637177+SunkenInTime@users.noreply.github.com> 2026-09-08 11:50:48-04:00 |
| committer |  GitHub <noreply@github.com> 2026-09-08 21:20:48+05:30 |
| log | 47eed9face4186635488fba5f2494eddd13f6491 |
| tree | 01662ea9c72ff295ec2f2ad48f0a4f94201d9d8e |
| parent | 061543e9e5b54ec0048725c37d52fef2962df173 |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): stop Windows terminal processes when closing (#10771)
2 files changed, 49 insertions(+), 11 deletions(-)
apps
server
src
apps/server/src/terminal/NodePtyAdapter.test.ts+43-8
| ... | ... | @@ -21,15 +21,50 @@ const spawn = vi.fn(() => ({ |
| 21 | 21 | |
| 22 | 22 | vi.mock("node-pty", () => ({ spawn })); |
| 23 | 23 | |
| 24 | const testLayer = NodePtyAdapter.layer.pipe( | |
| 25 | Layer.provide( | |
| 26 | Layer.mergeAll( | |
| 27 | NodeServices.layer, | |
| 28 | Layer.succeed(HostProcessPlatform, "win32"), | |
| 29 | Layer.succeed(HostProcessArchitecture, "x64"), | |
| 24 | const makeTestLayer = (platform: NodeJS.Platform = "win32") => | |
| 25 | NodePtyAdapter.layer.pipe( | |
| 26 | Layer.provide( | |
| 27 | Layer.mergeAll( | |
| 28 | NodeServices.layer, | |
| 29 | Layer.succeed(HostProcessPlatform, platform), | |
| 30 | Layer.succeed(HostProcessArchitecture, "x64"), | |
| 31 | ), | |
| 30 | 32 | ), |
| 31 | ), | |
| 32 | ); | |
| 33 | ); | |
| 34 | ||
| 35 | const testLayer = makeTestLayer(); | |
| 36 | ||
| 37 | for (const platform of ["win32", "linux", "darwin"] as const) { | |
| 38 | it.effect(`terminates through node-pty using ${platform} semantics`, () => | |
| 39 | Effect.gen(function* () { | |
| 40 | const adapter = yield* PtyAdapter.PtyAdapter; | |
| 41 | const process = yield* adapter.spawn({ | |
| 42 | shell: "test-shell", | |
| 43 | cwd: ".", | |
| 44 | cols: 80, | |
| 45 | rows: 24, | |
| 46 | env: {}, | |
| 47 | }); | |
| 48 | const nativeProcess = spawn.mock.results.at(-1)!.value; | |
| 49 | nativeProcess.kill.mockImplementation((signal?: string) => { | |
| 50 | if (platform === "win32" && signal) { | |
| 51 | throw new Error("Signals not supported on windows."); | |
| 52 | } | |
| 53 | }); | |
| 54 | ||
| 55 | process.kill("SIGTERM"); | |
| 56 | process.kill("SIGKILL"); | |
| 57 | process.kill(); | |
| 58 | ||
| 59 | assert.deepEqual( | |
| 60 | nativeProcess.kill.mock.calls, | |
| 61 | platform === "win32" | |
| 62 | ? [[undefined], [undefined], [undefined]] | |
| 63 | : [["SIGTERM"], ["SIGKILL"], [undefined]], | |
| 64 | ); | |
| 65 | }).pipe(Effect.provide(makeTestLayer(platform))), | |
| 66 | ); | |
| 67 | } | |
| 33 | 68 | |
| 34 | 69 | it.effect("spawns through the public adapter with the provided host references", () => |
| 35 | 70 | Effect.gen(function* () { |
apps/server/src/terminal/NodePtyAdapter.ts+6-3
| ... | ... | @@ -69,9 +69,11 @@ const ensureNodePtySpawnHelperExecutable = Effect.fn(function* () { |
| 69 | 69 | |
| 70 | 70 | class NodePtyProcess implements PtyAdapter.PtyProcess { |
| 71 | 71 | private readonly process: import("node-pty").IPty; |
| 72 | private readonly platform: NodeJS.Platform; | |
| 72 | 73 | |
| 73 | constructor(process: import("node-pty").IPty) { | |
| 74 | constructor(process: import("node-pty").IPty, platform: NodeJS.Platform) { | |
| 74 | 75 | this.process = process; |
| 76 | this.platform = platform; | |
| 75 | 77 | } |
| 76 | 78 | |
| 77 | 79 | get pid(): number { |
| ... | ... | @@ -87,7 +89,8 @@ class NodePtyProcess implements PtyAdapter.PtyProcess { |
| 87 | 89 | } |
| 88 | 90 | |
| 89 | 91 | kill(signal?: string): void { |
| 90 | this.process.kill(signal); | |
| 92 | // node-pty terminates the Windows process tree without a POSIX signal. | |
| 93 | this.process.kill(this.platform === "win32" ? undefined : signal); | |
| 91 | 94 | } |
| 92 | 95 | |
| 93 | 96 | onData(callback: (data: string) => void): () => void { |
| ... | ... | @@ -164,7 +167,7 @@ export const make = Effect.fn("NodePtyAdapter.make")(function* ( |
| 164 | 167 | cause, |
| 165 | 168 | }), |
| 166 | 169 | }); |
| 167 | return new NodePtyProcess(ptyProcess); | |
| 170 | return new NodePtyProcess(ptyProcess, platform); | |
| 168 | 171 | }), |
| 169 | 172 | }); |
| 170 | 173 | }); |