fix: kill daemon on windows without lsof

[SmashRex]

What & why

Fixes Windows daemon shutdown when lsof is unavailable.

Closes #1243

How it was verified

• Added coverage for using the daemon PID reported by /status when no listener or PID file is available.
• Added coverage for refusing to report success when /status identifies a daemon but no PID can be determined.
• Added coverage ensuring runKill does not report success when the killed process exits but the port remains occupied.
• Added the daemon PID to the /status payload.
• cli-kill.test.ts: 9 tests passed
• status-why.test.ts: 13 tests passed
• pnpm --filter @reticlehq/server lint: passed
• pnpm typecheck: passed
• git diff --check: passed
• Full pnpm test:unit: 75/76 tests passed; the remaining failure is the existing Windows symlink test in @reticlehq/cli-realm, which fails with EPERM when creating a symlink.

Gates run

• pnpm lint && pnpm typecheck && pnpm test:unit (~2 min — always) — test:unit is currently blocked by the Windows symlink EPERM failure described above.
• pnpm test:e2e (~8 min) — touched the tool surface, core, an observer, or telemetry
• pnpm gate:install (~15 min) — touched reticle init, vite-plugin, next, or babel-plugin
• pnpm test:e2e:desktop (~3 min) — touched adapters/realm/electron, adapters/realm/tauri, or desktop capture
• None of the above tiers apply to this change

Checklist

• Every commit is signed off (git commit -s) — CI's DCO check fails the PR without it.
• Tests added/updated (RED → GREEN); the change is covered by a test that would fail without it
• No any, no free strings (wire strings live in @reticlehq/core), no non-null !
• No console.log or internal tracking codes left in the diff
• Each changed file is under the 1000-line cap
• Docs updated, and a user-facing change adds a new file under .changes/ (never edit CHANGELOG.md — that file is assembled at release time, and editing it is what makes PRs conflict; format in .changes/README.md)
• Security-affecting? Auth/redaction/trust-boundary changes keep the localhost-only, no-app-data-leaves-the-machine, no-arbitrary-JS posture (usage telemetry stays anonymous + opt-out per docs/telemetry.md) and are covered by a test

[github-actions]

Thanks for your first pull request to Reticle! A maintainer aims to review within two days. Before then, pnpm verify locally catches most of what CI will, and CONTRIBUTING.md explains the rest. If CI does not start, a maintainer needs to approve it for first-time contributors; that is normal.

[greptile-apps]

Confidence Score: 3/5

[Medium risk] Adds daemon PID to status endpoint for Windows process termination.

The PR is not safe to merge until the status-derived PID cannot cause an unrelated process to be signaled and the named-constant requirement is met.

Findings

1.   Untrusted PID can be killed ▶
2.  Second status request can fail ▶
3.  New messages are free strings ▶

Summary

The PR adds the daemon PID to /status, uses it to locate a kill target when lsof and a PID file are unavailable, and checks the port again before reporting success.

• The status-derived PID needs to be bound to the process being terminated.
• The fallback makes a second status request that can independently fail.
• New user-facing and log strings should follow the repository’s named-constant rule.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["Probe TCP and /status"] --> B["Read PID file; look up listener"]
  B --> C{"Listener found?"}
  C -- Yes --> D["Use listener PID"]
  C -- No --> E{"Live recorded PID?"}
  E -- Yes --> F["Use recorded PID"]
  E -- No --> G["Fetch /status again for PID"]
  G --> H["Signal selected PID"]
  D --> H
  F --> H
  H --> I["Re-probe port before success"]

Loading

_{Reviews (1) · Last reviewed commit: "fix: clean up windows daemon kill change..."}

[greptile-apps]

Untrusted PID can be killed

On Windows or another system without lsof, a process that binds the configured port can serve a fake /status response containing an unrelated process’s PID. If there is no live recorded PID, this branch selects that value and runKill signals the unrelated process, even without --force. Checking that the value is a positive integer does not show that it belongs to the port holder.

How this was verified: The status probe accepts parsed HTTP JSON without authenticating its PID, and the no-listener path passes that PID to process signaling.

[greptile-apps]

Second status request can fail

The initial presence check already receives a /status response, but this fallback discards it and makes another request. If the second request times out or fails when neither lsof nor a live PID file is available, planKill refuses to stop the daemon despite the successful first response. Reusing that response would avoid the extra failure window and request.

[greptile-apps]

New messages are free strings

This new user-facing refusal message is inline rather than a named constant. The new survived-kill notes follow the same pattern. The repository requires every domain, wire, and UI string to be a named constant, so these messages need to satisfy that requirement before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[divshekhar]

This cannot enter the merge queue yet. CI shows a syntax error in server/src/index.ts at lines 592–594 (extra closing );), so formatting, TypeScript build, tests, and package quality fail. The DCO job also reports commit fcb680ca0b7c0ace591551ccb4bbafce904b2eec missing its Signed-off-by trailer, and Prettier flags .changes/1243-windows-daemon-kill.md. Please fix those three independent blockers, rerun pnpm verify, and push a signed-off revision.

[divshekhar]

The approach is sound: the netstat -ano fallback accepts LISTENING sockets only, refuses when no pid is known, and re-checks the port afterwards. The tests go red without the fix. Some cleanup is needed before merge:

• Dead code. The new pid on /status is never read, the status returned by probePresenceWithStatus is unused, and the new kill parameter on terminate has no caller. Either use the status pid or remove all three.
• Garbled comment around cli-kill.ts:230.
• Restored test. Bring back the safety test that loops over both force values ("never plans to kill without a pid").
• PR body. It mentions a "/status pid fallback" test that doesn't exist.

Note that CI's windows job is skipped on PRs, so netstat will first run in the merge queue.

[divshekhar]

Thanks for sticking with this. The netstat fallback is the right idea. The last review's items are still open on this head: the unused /status pid, probePresenceWithStatus's status and terminate's kill parameter (use them or remove all three), the comment near cli-kill.ts:230, the restored safety test that loops over both force values, and the PR description. One more thing: main already parses netstat -ano in server/src/command/setup/terminal/listeners.ts (parseNetstatListeners, with tests). Please reuse that rather than adding a second parser. If the directory-reach guard refuses cli -> setup, move the parser to a place both can import. Please rebase without the merge commit (the merge queue can't replay merges), then run pnpm verify.

[SmashRex]

Hi, thanks for the detailed feedback, and apologies for the delayed response.

I’ve been working through the issues and cleaning up the implementation. I’ve addressed the initial syntax, formatting, and DCO blockers, but I’m also going back through the remaining review comments, including reusing the existing netstat parser, restoring the safety test, removing the unused code, updating the PR description, and rebasing without the merge commit.

I’m checking the changes carefully and will rerun pnpm verify before confirming everything is ready. Thanks for your patience and for the guidance.

[divshekhar]

Thank you for the work. The dead code is gone, and the safety test that loops over both force values is back. These items remain.

1. Do not add a second netstat parser. parseNetstatPortHolder (server/src/command/cli/ports/port-holder.ts:133) copies parseNetstatListeners in server/src/command/setup/terminal/listeners.ts. Move parseNetstatListeners to a module that both cli/ports and setup/terminal can import. Then filter its rows by port.
2. Remove -p tcp at port-holder.ts:166. On Windows, netstat -p tcp shows only IPv4 sockets, so a daemon on [::1] is never found. Your IPv6 test passes only because it gives the parser the row directly. Main calls netstat -ano without -p.
3. Fix the broken comment at server/src/command/cli/cli-kill.ts:223-224.
4. Commits 48643cd58 and 850c399d0 merge main. The merge queue cannot replay them. Rebase onto main with git rebase origin/main and force-push.
5. Add a .changes/ entry. reticle kill on Windows is a user-facing change.

Then run pnpm verify.

[divshekhar]

Thank you. The shared netstat parser, the removed -p tcp and the .changes entry are good. Two items remain.

1. Item 3 of the last review: the broken comment is still at cli-kill.ts:221-223. Please fix it.
2. The branch has a copy of main's release commit 07a656a6 (the same patch). Run git fetch origin && git rebase origin/main to drop it, then force-push.