Greptile Summary

This PR fixes the long-standing Windows no-op in findGatewayPidsOnPortSync (which previously returned [] unconditionally on win32) by adding proper netstat-based PID detection and port polling. The approach is architecturally consistent with the existing Unix path and correctly routes through the new implementations at both call sites.

Issues found:

• P1 logic bug: The try/catch block around spawnSync("tasklist", …) in findGatewayPidsOnPortWindows is dead code. spawnSync never throws; errors are reported via res.error. If tasklist times out or fails, tl.stdout is empty and the PID is silently dropped — the opposite of the stated intent ("include the PID anyway — better to kill a stale process"). This should use an explicit tl.error || tl.status !== 0 check.
• P2 style: The original JSDoc comment block at lines 81–84 ("Find PIDs of gateway processes listening on the given port using synchronous lsof") was left stranded between the two new functions and is no longer attached to findGatewayPidsOnPortSync. It should be moved.

Confidence Score: 4/5

• Safe to merge after fixing the dead try/catch around tasklist — the happy path works correctly and the bug only affects the error-handling fallback.
• The PR correctly solves the Windows stub problem and the netstat-based approach is sound. One P1 logic bug exists: the try/catch around spawnSync("tasklist") is dead code and the intended fallback (include PID on failure) is silently broken. In practice tasklist rarely fails, so this won't block normal usage, but fixing it before merge is strongly recommended. The dangling JSDoc is cosmetic only. No data-loss or security risk.
• src/infra/restart-stale-pids.ts — specifically the tasklist error-handling block inside findGatewayPidsOnPortWindows

This is a comment left during a code review.
Path: src/infra/restart-stale-pids.ts
Line: 128-141

Comment:
**Dead `catch` block — fallback silently dropped**

`spawnSync` never throws. When `tasklist` times out or exits non-zero, `tl.error` is populated and `tl.stdout` is empty/null, but the `catch` block is never reached. The stated intent — _"If tasklist fails, include the PID anyway"_ — is silently broken: `tasklist` failures cause the PID to be **dropped** instead of conservatively included.

The fix is to check `tl.error` (or a non-zero/null status) explicitly:

```suggestion
    const tl = spawnSync("tasklist", ["/FI", `PID eq ${pid}`, "/FO", "CSV", "/NH"], {
        encoding: "utf8",
        timeout: spawnTimeoutMs,
        windowsHide: true,
      });
      if (tl.error || tl.status == null || tl.status !== 0) {
        // tasklist failed — include the PID anyway — better to kill a stale
        // process than leave a port occupied.
        confirmed.push(pid);
      } else if (tl.stdout && tl.stdout.toLowerCase().includes("node")) {
        confirmed.push(pid);
      }
```

Replace the whole `try/catch` block with the above (no `try` or `catch` wrappers needed).

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

---

This is a comment left during a code review.
Path: src/infra/restart-stale-pids.ts
Line: 81-84

Comment:
**Orphaned JSDoc block**

The original JSDoc comment (lines 81–84) was written for `findGatewayPidsOnPortSync`, but the new Windows function was inserted directly after it. The comment now has no function beneath it and is attached to nothing — TypeScript tooling will associate it with `findGatewayPidsOnPortWindows`, shadowing that function's own JSDoc. The original comment should either be moved to `findGatewayPidsOnPortSync` (where it belongs) or removed.

```suggestion
/**
 * Find PIDs of gateway processes listening on the given port using synchronous lsof.
 * Returns only PIDs that belong to openclaw gateway processes (not the current process).
 */
```

Move this block down to sit immediately above `export function findGatewayPidsOnPortSync` and remove it from its current position.

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

_{Reviews (1): Last reviewed commit: "fix(windows): detect stale gateway PIDs ..." | Re-trigger Greptile}

Addressed review feedback with two fixes:

Fix 1 (P1 logic bug): Replaced dead try/catch around spawnSync(tasklist) in findGatewayPidsOnPortWindows with an explicit tl.error || tl.status !== 0 check. spawnSync never throws, errors are reported via the .error property, so the catch block was unreachable dead code. Now errors are properly detected and the PID is included as intended.

Fix 2 (P2 orphaned JSDoc): Moved the stranded JSDoc comment from between the Windows function and findGatewayPidsOnPortSync to sit directly above findGatewayPidsOnPortSync where it belongs.

Pushed test fix: the win32 test was still expecting the old no-op behavior (return [], don't call spawnSync). Updated to mock netstat output and verify the Windows code path is actually exercised. The TypeError on line 99 (res undefined) should be resolved — CI will confirm.

Closing — this was superseded by #60480 which landed the same Windows stale-PID cleanup (netstat + command-line verification via isGatewayArgv) with a cleaner shared-helper architecture. All three review concerns (P1: dead catch, P1: broad node match, P2: skip unverified) are addressed in that implementation. Thanks @arifahmedjoy!

You are welcome, @andyk-ms!