Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changes/1306-windows-daemon-kill.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Fixed

- **`@reticlehq/server` � `reticle kill` could fail to identify a daemon holding a port on Windows.** Port detection now handles IPv4 and IPv6 listener output from `netstat`, helping Reticle identify the process to terminate.
101 changes: 92 additions & 9 deletions server/src/command/cli/cli-kill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,72 @@
* every platform while the signalling stays at the call site.
*/

import { describe, expect, it } from 'vitest';
import { KillAction, planKill } from './cli-kill.js';
import { describe, expect, it, vi } from 'vitest';
import { KillAction, planKill, runKill } from './cli-kill.js';

vi.mock('@/command/daemon/daemon.js', () => ({
isAlive: () => true,
readPid: () => null,
removePid: () => undefined,
}));

vi.mock('@/command/daemon/binding/port-presence.js', () => ({
PortPresence: {
DAEMON: 'daemon',
FOREIGN: 'foreign',
FREE: 'free',
},
probePresence: vi.fn(),
}));

vi.mock('@/command/daemon/binding/daemon-status-probe.js', () => ({
fetchStatus: vi.fn(),
}));

vi.mock('@/surface/mcp/mcp-proxy.js', () => ({
probeDaemon: vi.fn(),
}));

vi.mock('./ports/port-holder.js', () => ({
captureLookup: vi.fn(),
findPortHolder: vi.fn(),
}));

vi.mock('@/log.js', () => ({
log: vi.fn(),
}));

import { probePresence, PortPresence } from '@/command/daemon/binding/port-presence.js';
import { fetchStatus } from '@/command/daemon/binding/daemon-status-probe.js';
import { findPortHolder } from './ports/port-holder.js';

const DAEMON_PID = 70244;
const STRANGER_PID = 99001;

describe('planKill', () => {
it('has nothing to do when no listener and no live recorded pid', () => {
it('has nothing to do when no listener, no recorded pid, and no daemon status', () => {
expect(
planKill({ listener: null, recordedPid: null, answersStatus: false, force: false }).action,
planKill({
listener: null,
recordedPid: null,
answersStatus: false,
force: false,
}).action,
).toBe(KillAction.NOTHING);
});

it.each([false, true])('never plans to kill without a pid (force: %s)', (force) => {
const plan = planKill({
listener: null,
recordedPid: null,
answersStatus: true,
force,
});

expect(plan.action).not.toBe(KillAction.KILL);
expect(plan.pid).toBeUndefined();
});

it('kills the listener when it is the pid we recorded for the port', () => {
const plan = planKill({
listener: { pid: DAEMON_PID, command: 'node' },
Expand Down Expand Up @@ -97,10 +150,40 @@ describe('planKill', () => {
expect(plan.identifiedListener).toBe(false);
});

it('never plans to kill without a pid', () => {
for (const force of [false, true]) {
const plan = planKill({ listener: null, recordedPid: null, answersStatus: true, force });
expect(plan.action).not.toBe(KillAction.KILL);
}
it('refuses when /status identifies a daemon but no listener or recorded pid is available', () => {
const plan = planKill({
listener: null,
recordedPid: null,
answersStatus: true,
force: false,
});

expect(plan.action).toBe(KillAction.REFUSE);
expect(plan.pid).toBeUndefined();
});

it('does not report success when the killed process exits but the port is still occupied', async () => {
vi.mocked(probePresence).mockResolvedValueOnce(PortPresence.DAEMON);

vi.mocked(probePresence).mockResolvedValueOnce(PortPresence.FOREIGN);

vi.mocked(findPortHolder).mockReturnValue({
pid: DAEMON_PID,
command: 'node',
});

vi.mocked(fetchStatus).mockResolvedValue({ running: true, pid: DAEMON_PID });

const terminateProcess = vi.fn().mockResolvedValue({
gone: true,
escalated: false,
});

const result = await runKill(4400, false, terminateProcess);

expect(terminateProcess).toHaveBeenCalledWith(DAEMON_PID);
expect(fetchStatus).toHaveBeenCalledTimes(0);
expect(probePresence).toHaveBeenCalledTimes(2);
expect(result).toBe(false);
});
});
103 changes: 87 additions & 16 deletions server/src/command/cli/cli-kill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,37 @@ export function planKill(input: {
recordedPid: number | null;
/** Whether the port answers `/status`, i.e. whatever holds it is a Reticle daemon. */
answersStatus: boolean;
/** Whether the user has requested to force the kill. */
force: boolean;
}): KillPlan {
const { listener, recordedPid, answersStatus, force } = input;
if (null === listener) {
// No listener identified. On Windows and in slim containers that means the lookup could not run
// rather than that the port is free, so the recorded pid is the best target we have — and the
// report says which of the two it is rather than implying a lookup we never made.
if (null === recordedPid)
return { action: KillAction.NOTHING, forced: false, identifiedListener: false };
return { action: KillAction.KILL, pid: recordedPid, forced: false, identifiedListener: false };
// When the listener lookup is unavailable, /status can confirm that the port belongs to Reticle,
// but without a trusted process id there is no safe process to terminate.
if (null !== recordedPid) {
return {
action: KillAction.KILL,
pid: recordedPid,
forced: false,
identifiedListener: false,
};
}

// The port is known to be a Reticle daemon, but without a pid there is no safe process to kill.
if (answersStatus) {
return {
action: KillAction.REFUSE,
forced: false,
identifiedListener: false,
reason: PID_UNKNOWN_REFUSAL_REASON,
};
}

return {
action: KillAction.NOTHING,
forced: false,
identifiedListener: false,
};
}
const ours = answersStatus || listener.pid === recordedPid;
if (ours || force) {
Expand Down Expand Up @@ -145,10 +166,27 @@ async function terminate(pid: number): Promise<{ gone: boolean; escalated: boole
const PROXY_SPARED_NOTE =
'only the listener was signalled. Any `reticle mcp` proxy on this port holds a client connection, ' +
'not the port, and was left alone — it goes dormant and starts a fresh daemon on the next tool call.';
const PID_UNKNOWN_REFUSAL_REASON =
'The port answers /status as a Reticle daemon, but Reticle could not determine its pid. ' +
'Refusing to report success because the daemon is still running.';

const PID_SURVIVED_KILL_NOTE = (pid: number): string =>
`pid ${String(pid)} survived SIGKILL, so it is not yours to kill (permissions) or is an ` +
`unkillable zombie. The port is still held. ${PROXY_SPARED_NOTE}`;

const PID_EXITED_PORT_HELD_NOTE = (pid: number): string =>
`pid ${String(pid)} exited, but the port is still held. ${PROXY_SPARED_NOTE}`;

/** `reticle kill` — free the port. Resolves to whether the port is now free. */
export async function runKill(port: number, force: boolean): Promise<boolean> {
const presence = await probePresence(port, { tcpOpen: probeDaemon, status: fetchStatus });
export async function runKill(
port: number,
force: boolean,
terminateProcess: typeof terminate = terminate,
): Promise<boolean> {
const presence = await probePresence(port, {
tcpOpen: probeDaemon,
status: fetchStatus,
});
const recorded = readPid(port);
const plan = planKill({
listener: findPortHolder(port, captureLookup),
Expand All @@ -172,20 +210,53 @@ export async function runKill(port: number, force: boolean): Promise<boolean> {
log('reticle_kill_nothing_to_do', { port, presence });
return true;
}
const { gone, escalated } = await terminate(pid);
if (gone) removePid(port);
log(gone ? 'reticle_killed' : 'reticle_kill_survived', {

const { gone, escalated } = await terminateProcess(pid);

if (!gone) {
log('reticle_kill_survived', {
port,
pid,
escalated,
...(plan.forced ? { forced: true } : {}),
// A recorded PID is not necessarily the process observed listening on the port.
listenerIdentified: plan.identifiedListener,
note: PID_SURVIVED_KILL_NOTE(pid),
});
return false;
}

// A dead process does not by itself prove that the port is free. Another process may still
// hold the port, so verify the port state before reporting a successful kill.
const remainingPresence = await probePresence(port, {
tcpOpen: probeDaemon,
status: fetchStatus,
});

if (remainingPresence !== PortPresence.FREE) {
log('reticle_kill_survived', {
port,
pid,
escalated,
...(plan.forced ? { forced: true } : {}),
listenerIdentified: plan.identifiedListener,
note: PID_EXITED_PORT_HELD_NOTE(pid),
});
return false;
}

removePid(port);

log('reticle_killed', {
port,
pid,
escalated,
...(plan.forced ? { forced: true } : {}),
// Say where the pid came from. Without `lsof` this is the pid we recorded, not the pid we
// observed listening, and those are different claims.
listenerIdentified: plan.identifiedListener,
note: gone
? PROXY_SPARED_NOTE
: `pid ${String(pid)} survived SIGKILL, so it is not yours to kill (permissions) or is an ` +
`unkillable zombie. The port is still held. ${PROXY_SPARED_NOTE}`,
note: PROXY_SPARED_NOTE,
});
return gone;

return true;
}
40 changes: 39 additions & 1 deletion server/src/command/cli/ports/port-holder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { describe, expect, it } from 'vitest';
import {
describeForeignHolder,
findListenerCwd,
parseNetstatPortHolder,
parsePortHolder,
parseProcessCwd,
} from './port-holder.js';

/**
* Naming the process that holds the port.
*
Expand Down Expand Up @@ -51,6 +51,44 @@ describe('parsePortHolder', () => {
});
});

describe('parseNetstatPortHolder', () => {
it('reads the pid of a TCP listener for the requested port', () => {
const stdout = ' TCP 127.0.0.1:4400 0.0.0.0:0 LISTENING 90502\n';

expect(parseNetstatPortHolder(stdout, 4400)).toEqual({
pid: 90502,
command: 'unknown',
});
});

it('matches IPv6 listeners', () => {
const stdout = ' TCP [::1]:4400 [::]:0 LISTENING 90777\n';

expect(parseNetstatPortHolder(stdout, 4400)).toEqual({
pid: 90777,
command: 'unknown',
});
});

it('ignores listeners on other ports', () => {
const stdout = ' TCP 127.0.0.1:4500 0.0.0.0:0 LISTENING 90502\n';

expect(parseNetstatPortHolder(stdout, 4400)).toBeNull();
});

it('ignores non-listening connections', () => {
const stdout = ' TCP 127.0.0.1:4400 127.0.0.1:5500 ESTABLISHED 90502\n';

expect(parseNetstatPortHolder(stdout, 4400)).toBeNull();
});

it('returns null when the listener pid is invalid', () => {
const stdout = ' TCP 127.0.0.1:4400 0.0.0.0:0 LISTENING not-a-pid\n';

expect(parseNetstatPortHolder(stdout, 4400)).toBeNull();
});
});

describe('describeForeignHolder', () => {
it('names the pid and command, and does not suggest the command that kills the agent', () => {
const text = describeForeignHolder(4400, { pid: 90502, command: 'node' });
Expand Down
14 changes: 13 additions & 1 deletion server/src/command/cli/ports/port-holder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { spawnSync } from 'node:child_process';
import { parseNetstatListeners } from '../../netstat/listeners.js';

export interface PortHolder {
pid: number;
Expand Down Expand Up @@ -129,6 +130,12 @@ export function describeForeignHolder(
);
}

/** Find the first TCP listener for a port in `netstat -ano` output. */
export function parseNetstatPortHolder(stdout: string, port: number): PortHolder | null {
const listener = parseNetstatListeners(stdout).find((row) => row.port === port);
return listener === undefined ? null : { pid: listener.pid, command: 'unknown' };
}

/** The lookup itself. `exec` is injected so the shelling out never runs in a unit test. */
export function findPortHolder(
port: number,
Expand All @@ -138,7 +145,12 @@ export function findPortHolder(
// that matters: without it this also reports the agent's own MCP proxy, which is a client of the
// port, not its owner.
const out = exec('lsof', ['-nP', `-iTCP:${String(port)}`, '-sTCP:LISTEN', '-F', 'pc']);
return null === out ? null : parsePortHolder(out);
if (null !== out) return parsePortHolder(out);

// Windows does not normally have lsof. netstat is built into Windows and reports the owning PID
// for listening TCP sockets, giving kill a process identity tied to the actual port owner.
const netstat = exec('netstat', ['-ano']);
return null === netstat ? null : parseNetstatPortHolder(netstat, port);
}

/** `lsof -Fn` emits the file name on its own line, prefixed `n`. */
Expand Down
37 changes: 37 additions & 0 deletions server/src/command/netstat/listeners.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/** One TCP listener and the process that owns it. */
export interface NetstatListener {
readonly port: number;
readonly pid: number;
}

const NETSTAT_PROTO = 'TCP';
const NETSTAT_STATE = 'LISTENING';
const NETSTAT_MIN_COLUMNS = 5;

/**
* Parse Windows `netstat -ano` output.
*
* Read the port after the LAST colon so IPv6 addresses such as `[::]:3000`
* are handled correctly. Do not restrict netstat to IPv4 with `-p tcp`.
*/
export function parseNetstatListeners(out: string): NetstatListener[] {
const rows: NetstatListener[] = [];

for (const line of String(out).split(/\r?\n/)) {
const fields = line.trim().split(/\s+/);
if (fields.length < NETSTAT_MIN_COLUMNS) continue;
if (fields[0]?.toUpperCase() !== NETSTAT_PROTO || fields[3]?.toUpperCase() !== NETSTAT_STATE) {
continue;
}

const localAddress = fields[1] ?? '';
const port = Number(localAddress.slice(localAddress.lastIndexOf(':') + 1));
const pid = Number(fields[4]);

if (Number.isInteger(port) && port > 0 && Number.isInteger(pid) && pid > 0) {
rows.push({ port, pid });
}
}

return rows;
}
Loading
Loading