Skip to content

bug: watch's pid-tracker kills any process matching a tracked PID, no identity check #1641

Closed

Description

@omercangumus

Summary

PidTracker.cleanupStale() (packages/squad-cli/src/cli/commands/watch/pid-tracker.ts:84-106) kills every PID in .watch-pids that's still alive, using nothing but process.kill(pid, 0) to decide "still alive" and killTree(pid) to kill it. TrackedProcess stores name and spawnedAt, but neither is ever compared against the real process before the kill — they're only used in the log line after the kill already happened. Same for cleanup() (line 65-80), same mechanism.

Proven (verified below, this run): the kill path is bare PID-number equality with zero identity check — no cmdline comparison, no start-time comparison, no owner check. A tracked record can claim a spawn time a month in the past for a process that actually started 90 seconds ago, and cleanupStale() kills it anyway without any contradiction check.

Reasoned, not simulated on this machine: on a real crash where watch dies without cleaning up its .watch-pids file, followed by a reboot (or just enough process churn for the OS to cycle back through the PID space), the OS can reassign that same PID number to a completely unrelated process before watch restarts and calls cleanupStale(). Given the proven behavior above, that unrelated process would be killed. I can't trigger a real PID-space wraparound + reassignment on this machine to observe that exact end-to-end chain, so I'm not claiming to have reproduced that half — only that the code has no defense against it if it happens.

Steps to reproduce

Verified on dev @ a4b60e8, Windows 11, Node v22.21.0.

Started a real, long-running process to stand in for "some unrelated process that now happens to occupy a given PID":

node -e "console.log('sleeper started, pid=' + process.pid, 'startedAt=' + new Date().toISOString()); setTimeout(()=>{}, 300000);"
// sleeper started, pid=26820 startedAt=2026-08-07T23:19:00.128Z

Confirmed independently via the OS, not just the process's own claim:

PS> Get-Process -Id 26820 | Select-Object Id,ProcessName,StartTime
   Id ProcessName StartTime
   -- ----------- ---------
26820 node        8.08.2026 02:19:00

Wrote a .watch-pids file claiming this PID belongs to an entry from a month ago:

[
  {
    "pid": 26820,
    "name": "totally-unrelated-agent-spawn-from-last-week",
    "spawnedAt": "2026-07-01T00:00:00.000Z"
  }
]

Ran PidTracker.cleanupStale() directly against that team root (compiled module, no mocks):

ALIVE BEFORE: true
⚠️ Killed orphaned process: totally-unrelated-agent-spawn-from-last-week (PID 26820, spawned 2026-07-01T00:00:00.000Z)
cleanupStale() returned: 1
ALIVE AFTER: false

The process's real start time (2026-08-08 02:19:00, confirmed by Get-Process) flatly contradicts the tracked record's claimed spawnedAt (2026-07-01, over a month earlier). cleanupStale() never checks this — it killed the process purely because the PID number matched.

Root cause

packages/squad-cli/src/cli/commands/watch/pid-tracker.ts:

  • isAlive() (line 38-45): process.kill(pid, 0) — existence check only, no identity.
  • killTree() (line 48-62): kills by PID, no identity check.
  • cleanupStale() (line 84-106) and cleanup() (line 65-80): both call isAlive then killTree in sequence with no code path that reads proc.name or proc.spawnedAt before killing — those two fields exist only for the post-kill log message at line 96.

Suggested fix direction

Before killing a PID from a stale/tracked record, cross-check something about the live process against what was recorded — process start time is available cross-platform (wmic process where ProcessId=X get CreationDate / /proc/<pid>/stat starttime / ps -o lstart=) and would catch exactly the contradiction demonstrated above. If the live process's actual start time doesn't match spawnedAt (within some tolerance), skip the kill and log a warning instead of silently trusting the PID number.

Activity

  1. omercangumus commented on Aug 9, 2026

    @omercangumus
    ContributorAuthor

    picking this up. adding a start-time cross-check before any kill — cross-platform via wmic on windows / ps -o lstart= elsewhere, compared against the tracked spawnedAt with a tolerance window. if the OS start time contradicts the record we skip the kill and log a warning instead of trusting the bare PID. if start time can't be determined at all, falls back to today's behavior so normal orphan cleanup doesn't regress. PR shortly

  2. added a commit that references this issue on Aug 23, 2026
    9ede019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions