diff --git a/config/patches/node-pty@1.1.0.patch b/config/patches/node-pty@1.1.0.patch index 4e75a8f151e..34703560ef2 100644 --- a/config/patches/node-pty@1.1.0.patch +++ b/config/patches/node-pty@1.1.0.patch @@ -430,7 +430,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d } #endif diff --git a/src/win/conpty.cc b/src/win/conpty.cc -index 7b286d3d644c26141df516929703aa6e129df4b2..4ae9ddec6ac3e9fdaa380000d3017bbb394f22d6 100644 +index 7b286d3d644c26141df516929703aa6e129df4b2..a1a63d6380ba5b0a669339435064b9f51e0706dc 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -46,6 +46,11 @@ struct pty_baton { @@ -449,9 +449,9 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4ae9ddec6ac3e9fdaa380000d3017bbb GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); // Clean up handles CloseHandle(baton->hShell); -+ // Orca: closing the last job handle is what reaps whatever the shell left -+ // behind. Without this the job outlives the pane and its descendants keep -+ // running (and keep holding the worktree directory open). ++ // Orca: release the job once the shell is gone. Without kill-on-close this ++ // only frees the handle -- anything the user backgrounded is orphaned, as ++ // it was before this patch. + if (baton->hJob != nullptr) { + CloseHandle(baton->hJob); + baton->hJob = nullptr; @@ -480,14 +480,14 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..4ae9ddec6ac3e9fdaa380000d3017bbb + // descendant that reparented, which is why detached agent children outlived + // their pane and held the worktree directory open. + // -+ // JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE means the tree also dies if this host -+ // process dies without unwinding, so a crashed daemon cannot strand shells. ++ // Deliberately WITHOUT JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE. Measured on ++ // Windows 11: with that flag, closing the handle when the shell exits also ++ // kills whatever the user left running, so typing `exit` in a pane reaped a ++ // `start /b` server that used to survive. This job exists to make an ++ // EXPLICIT teardown exact, not to redefine what a clean exit means. + HANDLE hJob = CreateJobObjectW(nullptr, nullptr); + if (hJob != nullptr) { -+ JOBOBJECT_EXTENDED_LIMIT_INFORMATION jobLimits{}; -+ jobLimits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; -+ if (!SetInformationJobObject(hJob, JobObjectExtendedLimitInformation, &jobLimits, sizeof(jobLimits)) || -+ !AssignProcessToJobObject(hJob, piClient.hProcess)) { ++ if (!AssignProcessToJobObject(hJob, piClient.hProcess)) { + // Why tolerate failure: an outer job without JOB_OBJECT_LIMIT_BREAKAWAY_OK + // (some EDR and container hosts) refuses the assignment. The pty must + // still start; ownership just degrades to the older best-effort path. diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index 9e5624c3924..a96bb9955bf 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -104,11 +104,17 @@ parent-pid walk nor `GetConsoleProcessList` sees that grandchild — it leaves the console and reparents, which is what left `claude.exe`/`node.exe`/`cmd.exe` holding worktree directories open (#9045, #10475, #10897). -`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` is set, so the tree is also reaped if the -hosting process dies without unwinding. The job belongs to the **terminal -daemon**, not to the app: an app-main crash leaves sessions alive (asserted by -`.github/workflows/win-crash-survival-e2e.yml`), while a daemon death now reaps -its shells instead of stranding them (#9195, #10415). +The per-PTY job deliberately does **not** set +`JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`. Measured on Windows 11: with that flag, +releasing the handle when the shell exits also kills whatever the user left +running, so typing `exit` in a pane reaped a `start /b` server that used to +survive. The job exists to make an *explicit* teardown exact, not to redefine +what a clean exit means. + +That means reaping a dead daemon's shells (#9195, #10415) is **not** delivered +by this job. It needs a separate daemon-level job that the daemon assigns +itself to at startup; children inherit job membership, so its closure on daemon +death reaps the shells without touching clean-exit semantics. Once the shell exits, node-pty drops its handle record and closes the job, so a terminated tree reports `null` rather than `[]`. Null means *unverifiable* in diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cbd1d443102..34821c1c7f1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,7 +27,7 @@ patchedDependencies: hash: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673 path: config/patches/lint-staged@16.4.0.patch node-pty@1.1.0: - hash: 8fc49f17011b6611a5b8c00e83a6f12e14e75aada2b0ef26dc5393f8376d20e8 + hash: 65d30811c4f422be2637e01f1ad25c385ddc6b59d5c90309e5656964dda192b0 path: config/patches/node-pty@1.1.0.patch importers: @@ -69,7 +69,7 @@ importers: version: 3.3.1 node-pty: specifier: ^1.1.0 - version: 1.1.0(patch_hash=8fc49f17011b6611a5b8c00e83a6f12e14e75aada2b0ef26dc5393f8376d20e8) + version: 1.1.0(patch_hash=65d30811c4f422be2637e01f1ad25c385ddc6b59d5c90309e5656964dda192b0) posthog-node: specifier: ^5.33.3 version: 5.33.3 @@ -12046,7 +12046,7 @@ snapshots: node-int64@0.4.0: {} - node-pty@1.1.0(patch_hash=8fc49f17011b6611a5b8c00e83a6f12e14e75aada2b0ef26dc5393f8376d20e8): + node-pty@1.1.0(patch_hash=65d30811c4f422be2637e01f1ad25c385ddc6b59d5c90309e5656964dda192b0): dependencies: node-addon-api: 7.1.1 diff --git a/src/main/windows/windows-pty-job.ts b/src/main/windows/windows-pty-job.ts index a1f16b59587..bd6211ecfad 100644 --- a/src/main/windows/windows-pty-job.ts +++ b/src/main/windows/windows-pty-job.ts @@ -18,6 +18,10 @@ import { createRequire } from 'node:module' * A job object replaces the inference with a handle. node-pty puts each shell * into its own job at creation, before it can spawn anything, so membership is * the kernel's answer rather than ours. + * + * The job is not kill-on-close: it makes an explicit teardown exact, and + * deliberately does not change what a clean shell exit means for anything the + * user backgrounded. */ const requireFromMain = createRequire(__filename)