fix(tauri): own Windows CLI subtree with a job object

[pascalandr]

Summary

• Follow-up to fix(tauri): force Windows process tree shutdown #240 to make Windows desktop shutdown reliable this time, even when the tracked CLI wrapper PID exits before its descendants
• Attach the spawned CLI process to a Windows Job Object with KILL_ON_JOB_CLOSE, so the desktop app owns the whole subtree instead of relying only on taskkill /PID <wrapper> /T
• Keep the current graceful-then-force shutdown path, but add a robust OS-level fallback that reaps orphaned workspace processes when the wrapper is already gone

Root Cause

The previous Windows shutdown logic still depended on the PID tracked by Tauri. In practice that PID can be a short-lived Node wrapper. Once that wrapper exits, taskkill can report success or PID-not-found while descendants remain alive, and the desktop app no longer has a reliable handle to reap them.

Validation

• cargo check --manifest-path packages/tauri-app/src-tauri/Cargo.toml
• cargo build --release --manifest-path packages/tauri-app/src-tauri/Cargo.toml
• Manual local test: orphaned processes are cleaned up after desktop shutdown

[shantur]

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

[codenomadbot]

I reviewed the latest PR state as a gatekeeper pass.

I understood the request to be: review this PR for regressions, look for a stronger implementation if one exists, and call out anything that should block merge.

Findings

1. packages/tauri-app/src-tauri/Cargo.toml:15-28 adds a batch of new cross-platform dependencies (axum, axum-server, bytes, futures-util, rcgen, rustls, reqwest, rand, tokio) that do not appear to be used by the job-object change in cli_manager.rs.

   This is the one thing I would block on. The Windows job-object work itself is narrowly scoped, but these manifest additions materially widen the build and supply-chain surface for every platform, and they caused Cargo to try to lock 40+ extra transitive crates locally. For this PR, the only manifest delta that looks justified is the extra windows-sys features needed for Job Objects. If these Rust deps are for separate upcoming work, they should land in the PR that actually uses them.

Review outcome

• I did not find a higher-confidence regression in the cli_manager.rs job-object lifecycle itself.
• I did not find a clearly better implementation that would justify rejecting the job-object approach; using KILL_ON_JOB_CLOSE is the right OS-level direction for the orphan-wrapper problem.
• I would still request the manifest be trimmed back to only the Windows API additions needed by this change.

Validation note

• I diffed the PR against origin/dev and reviewed the changed Rust paths directly.
• I attempted cargo check --manifest-path packages/tauri-app/src-tauri/Cargo.toml, but in this environment the Tauri build script fails before compile because packages/tauri-app/src-tauri/tauri.conf.json references resources/server, which is not present until the bundle step has populated it. So I could not independently reproduce the PR’s claimed Cargo validation from this checkout alone.

--
Yours,
CodeNomadBot

[shantur]

@CodeNomadBot

Do we need to align packages/electron-app similar to this change on Windows?

[codenomadbot]

I understood the question to be: should packages/electron-app get the same Windows job-object style hardening as this Tauri PR?

My answer is: not right now.

Why:

• In packages/electron-app/electron/main/process-manager.ts:187-198, Windows uses a direct spawn(...) path with detached = false and supportsUserShell() is disabled on Windows.
• packages/electron-app/electron/main/user-shell.ts:52-54 makes that explicit: user-shell launching is POSIX-only.
• The exact bug this Tauri PR is fixing is that Tauri can end up tracking a short-lived wrapper PID on Windows. Electron’s Windows path is not doing that today; it launches Node directly against the CLI entry (process-manager.ts:579-585, 638-648).
• Electron’s current Windows stop path is still taskkill /PID <pid> /T (process-manager.ts:331-360), but because the tracked PID is the actual long-lived Node CLI process rather than a transient wrapper shell, it is not the same risk profile as the Tauri path this PR addresses.

So my gatekeeper take is:

• No immediate alignment is required for packages/electron-app on Windows based on this PR alone.
• If Electron later starts launching the CLI through an extra Windows wrapper layer again, or if we observe orphaned descendants after the direct Node CLI exits, then yes, we should consider the same kind of OS-owned subtree mechanism there too.

Small caveat:

• Electron is still relying on taskkill rather than an OS-owned handle for the subtree, so Tauri’s new approach is technically more robust in the abstract.
• I just do not see evidence in the current Electron Windows launch path that it needs to be bundled into this PR.

--
Yours,
CodeNomadBot

[pascalandr]

Sorry, I had accidentally included unrelated manifest changes in the PR earlier. I've cleaned that up now.

[shantur]

@CodeNomadBot

Can you suggest a patch for packages/electron-app to match Tauri behavior on Windows

[codenomadbot]

I understood the request to be: suggest a concrete patch for packages/electron-app so Windows ownership of the CLI subtree matches the new Tauri behavior as closely as possible.

My recommendation is:

• Do not try to force this purely inside the current Node/Electron process-manager.ts.
• Add a tiny Windows-only helper supervisor that creates a Job Object with KILL_ON_JOB_CLOSE, spawns the real CLI process, assigns it to the job, and stays alive holding the job handle.
• Then have Electron track and stop that helper process, not the raw CLI child.

Why this is the right patch:

• Tauri can do this because it already has direct Win32 access in Rust.
• packages/electron-app currently has no native FFI / N-API layer for Win32 Job Objects.
• Plain Node spawn() plus taskkill cannot give Electron the same OS-owned subtree guarantee that KILL_ON_JOB_CLOSE gives Tauri.
• So if the goal is truly “match Tauri behavior”, the minimal honest patch is a tiny native helper, not more TypeScript around taskkill.

Suggested patch shape

1. Add a small Windows helper binary for Electron packaging

• New helper, for example packages/electron-app/electron/resources/windows-cli-job-supervisor.exe
• Responsibility:
  • accept { command, args, cwd }
  • create Job Object
  • set JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
  • spawn the real CLI process
  • assign the child to the job
  • proxy stdout/stderr
  • exit when asked to stop or when parent disconnects

This is the same ownership model as the Tauri patch: the long-lived supervisor owns the job handle, so if Electron kills the supervisor or the supervisor dies unexpectedly, Windows reaps the whole subtree.

2. Route Windows launches through the supervisor

In packages/electron-app/electron/main/process-manager.ts, change the Windows launch path so it uses the helper instead of directly spawning node <cli-entry>.

Conceptually:

if (process.platform === "win32") {
  const supervisorPath = this.resolveWindowsJobSupervisorPath()
  const payload = JSON.stringify({
    command: process.execPath,
    args: cliEntry.runner === "tsx"
      ? [cliEntry.runnerPath!, cliEntry.entry, ...args]
      : [cliEntry.entry, ...args],
    cwd: process.cwd(),
  })

  child = utilityProcess.fork(supervisorPath, [payload], {
    stdio: "pipe",
    serviceName: "CodeNomad CLI Job Supervisor",
  })
  this.childLaunchMode = "utility"
} else {
  // keep existing path
}

If utilityProcess.fork() cannot be used with the chosen helper format, plain spawn() of the helper is also fine. The important part is: Electron tracks the helper, and the helper tracks the job + real CLI.

3. Use helper shutdown as the kill switch

The stop path then becomes simpler and more reliable on Windows:

• graceful stop: ask the helper to terminate its child
• force stop: kill the helper process
• killing the helper drops the Job Object handle
• Windows kills the entire subtree automatically

That means taskkill /PID <cliPid> /T becomes fallback-only, not the primary ownership mechanism.

Where to patch

• packages/electron-app/electron/main/process-manager.ts
  • add Windows helper launch path
  • resolve helper path from packaged/dev resources
  • treat helper as the tracked child on Windows
• packages/electron-app/electron/resources/
  • add the helper binary/script source and bundle it
• packages/electron-app/package.json
  • include the helper in extraResources if needed

Minimal API for the helper

Input:

{
  "command": "C:\\Path\\to\\node.exe",
  "args": ["C:\\Path\\to\\server\\dist\\bin.js", "serve", "--host", "127.0.0.1"],
  "cwd": "C:\\workspace\\CodeNomad"
}

Behavior:

• create job
• spawn child suspended or normally
• assign child to job immediately
• resume if needed
• pipe stdout/stderr back to parent
• on helper shutdown, terminate child gracefully if possible
• on forced shutdown or crash, rely on job close to reap descendants

My gatekeeper recommendation

If you want Electron to truly match Tauri’s Windows semantics, this is the patch I would endorse.

I would not endorse a TypeScript-only patch that just adds more taskkill calls, because that still would not match the key Tauri property: an OS-owned handle to the whole subtree.

Small suggestion, not part of this PR:

• If you want to minimize stack sprawl, the helper could be a tiny Rust sidecar since the repo already has Rust in the Tauri app and the exact Job Object logic already exists there conceptually.
• That is cleaner than introducing a new Node native binding dependency just for Electron.

--
Yours,
CodeNomadBot

[shantur]

Thanks @pascalandr