Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
4 changes: 2 additions & 2 deletions .github/workflows/managed-images.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ jobs:
CANDIDATE_SHA: ${{ github.event.pull_request.head.sha }}
# Retains the reviewed discovery-permission repair and the current
# managed-image security inventory. The previous staging source pinned
# Vim 9.2.0782, which cannot satisfy the candidate's 9.2.0858 contract.
STAGING_QA_SOURCE_SHA: af2a73f0d6ce8f08a2975560f376470387c535d0
# libssh2 nemoclaw1, which cannot satisfy the candidate's nemoclaw2 contract.
STAGING_QA_SOURCE_SHA: ce96811ddb418ad01c040521a1fe912b5bcb405e
STAGING_QA_BASE_IMAGE: nemoclaw-deepagents-code-base:staging-31396519688
STAGING_QA_FINAL_IMAGE: nemoclaw-managed-pr/langchain-deepagents-code-staging-qa
steps:
Expand Down
164 changes: 162 additions & 2 deletions src/lib/onboard/model-router-process.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { describe, expect, it } from "vitest";
import { describe, expect, it, vi } from "vitest";

import { findModelRouterPidForPort } from "./model-router-process";
import { findModelRouterPidForPort, stopModelRouterProcess } from "./model-router-process";

const ROUTER_ARGS = ["/opt/model-router", "proxy", "--port", "4000"];

describe("findModelRouterPidForPort", () => {
it("returns the PID when a model-router proxy is found via direct proc scan (#5169)", () => {
Expand Down Expand Up @@ -65,3 +67,161 @@ describe("findModelRouterPidForPort", () => {
expect(pid).toBe(100);
});
});

describe("stopModelRouterProcess", () => {
it("returns when the recorded PID does not report as running and the health endpoint is not healthy", async () => {
const isHealthy = vi.fn(async () => false);
const kill = vi.fn();

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => false,
isHealthy,
kill,
}),
).resolves.toBeUndefined();

expect(isHealthy).toHaveBeenCalledWith(4000, 1000);
expect(kill).not.toHaveBeenCalled();
});

it("refuses replacement when the recorded PID does not report as running but the health endpoint remains healthy", async () => {
const kill = vi.fn();

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => false,
isHealthy: async () => true,
kill,
}),
).rejects.toThrow("PID 123 no longer reports as running but port 4000 remains healthy");

expect(kill).not.toHaveBeenCalled();
});

it("returns only after the recorded PID does not report as running and the health endpoint is not healthy", async () => {
let running = true;
let healthy = true;
const signals: NodeJS.Signals[] = [];

await stopModelRouterProcess(123, 4000, {
isRunning: () => running,
readCommandLine: () => ROUTER_ARGS,
isHealthy: async () => healthy,
kill: (_pid, signal) => {
signals.push(signal);
running = false;
healthy = false;
},
sleep: async () => {},
});

expect(signals).toEqual(["SIGTERM"]);
});

it("refuses to signal a PID that no longer belongs to the router", async () => {
const signals: NodeJS.Signals[] = [];

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => true,
readCommandLine: () => ["/usr/bin/unrelated-service", "--port", "4000"],
isHealthy: async () => true,
kill: (_pid, signal) => signals.push(signal),
sleep: async () => {},
}),
).rejects.toThrow("it is not the model-router proxy");
expect(signals).toEqual([]);
});

it("fails closed when SIGTERM cannot be delivered", async () => {
await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => true,
readCommandLine: () => ROUTER_ARGS,
isHealthy: async () => true,
kill: () => {
throw new Error("EPERM");
},
sleep: async () => {},
}),
).rejects.toThrow("could not send SIGTERM");
});

it("does not escalate when a process survives SIGTERM without a PID-stable handle", async () => {
const signals: NodeJS.Signals[] = [];

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => true,
readCommandLine: () => ROUTER_ARGS,
isHealthy: async () => true,
kill: (_pid, signal) => signals.push(signal),
sleep: async () => {},
}),
).rejects.toThrow("refuses PID-based SIGKILL");
expect(signals).toEqual(["SIGTERM"]);
});

it("sends no escalation signal when PID ownership changes during graceful shutdown", async () => {
let ownershipChecks = 0;
const signals: NodeJS.Signals[] = [];

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => true,
readCommandLine: () => {
ownershipChecks += 1;
return ownershipChecks === 1 ? ROUTER_ARGS : ["/usr/bin/unrelated-service"];
},
isHealthy: async () => false,
kill: (_pid, signal) => signals.push(signal),
sleep: async () => {},
}),
).rejects.toThrow("ownership changed during shutdown");
expect(signals).toEqual(["SIGTERM"]);
});

it("does not send SIGKILL when a replacement owns the PID at the final command-line check", async () => {
let ownershipChecks = 0;
let replacementOwnsPid = false;
const routerSignals: NodeJS.Signals[] = [];
const replacementSignals: NodeJS.Signals[] = [];

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => true,
readCommandLine: () => {
ownershipChecks += 1;
replacementOwnsPid ||= ownershipChecks === 2;
return ROUTER_ARGS;
},
isHealthy: async () => true,
kill: (_pid, signal) => {
(replacementOwnsPid ? replacementSignals : routerSignals).push(signal);
},
sleep: async () => {},
}),
).rejects.toThrow("refuses PID-based SIGKILL");

expect(ownershipChecks).toBe(2);
expect(routerSignals).toEqual(["SIGTERM"]);
expect(replacementSignals).toEqual([]);
});

it("does not report success when the PID does not report as running but the health endpoint remains healthy", async () => {
let running = true;

await expect(
stopModelRouterProcess(123, 4000, {
isRunning: () => running,
readCommandLine: () => ROUTER_ARGS,
isHealthy: async () => true,
kill: () => {
running = false;
},
sleep: async () => {},
}),
).rejects.toThrow("port 4000 remains healthy");
});
});
82 changes: 69 additions & 13 deletions src/lib/onboard/model-router-process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ export type ModelRouterProcessOwnershipDeps = {
readCommandLine?: (pid: number) => string[] | null;
};

export type StopModelRouterProcessDeps = ModelRouterProcessOwnershipDeps & {
isHealthy?: (port: number, timeoutMs?: number) => Promise<boolean>;
kill?: (pid: number, signal: NodeJS.Signals) => void;
sleep?: (delayMs: number) => Promise<void>;
};

type ModelRouterCommandLineReaderDeps = {
readProcCommandLine?: (pid: number) => string[] | null;
readPsCommandLine?: (pid: number) => string[] | null;
Expand Down Expand Up @@ -118,25 +124,75 @@ export function doesModelRouterProcessOwnPort(
return Array.isArray(args) && isModelRouterCommandLineForPort(args, port);
}

export async function stopModelRouterProcess(pid: number, port: number): Promise<void> {
/**
* Stop the recorded Model Router process and return only after its PID no
* longer reports as running and its health endpoint is not healthy. The
* session stores a numeric PID, not a PID-stable OS handle. Ownership
* validation and SIGTERM delivery are separate OS operations, so PID reuse can
* still redirect SIGTERM. Never send SIGKILL without a PID-stable handle.
*/
export async function stopModelRouterProcess(
pid: number,
port: number,
deps: StopModelRouterProcessDeps = {},
): Promise<void> {
const isRunning = deps.isRunning ?? isProcessRunning;
const readCommandLine = deps.readCommandLine ?? readModelRouterProcessCommandLine;
const isHealthy = deps.isHealthy ?? isRouterHealthy;
const kill = deps.kill ?? ((targetPid, signal) => process.kill(targetPid, signal));
const sleep =
deps.sleep ?? ((delayMs) => new Promise<void>((resolve) => setTimeout(resolve, delayMs)));

if (!isRunning(pid)) {
if (!(await isHealthy(port, 1000))) return;
throw new Error(
`NemoClaw refuses to replace the Model Router: recorded PID ${pid} no longer reports as running but port ${port} remains healthy.`,
);
}
if (
!doesModelRouterProcessOwnPort(pid, port, {
isRunning,
readCommandLine,
})
) {
throw new Error(
`NemoClaw refuses to stop PID ${pid}: it is not the model-router proxy for port ${port}.`,
);
}

try {
process.kill(pid, "SIGTERM");
} catch {
return;
kill(pid, "SIGTERM");
} catch (error) {
if (!isRunning(pid) && !(await isHealthy(port, 1000))) return;
throw new Error(
`NemoClaw could not send SIGTERM to Model Router PID ${pid}: ${
error instanceof Error ? error.message : String(error)
}`,
);
}
for (let _attempt = 0; _attempt < 10; _attempt++) {
await new Promise((resolve) => setTimeout(resolve, 500));
if (!isProcessRunning(pid) && !(await isRouterHealthy(port, 1000))) return;
await sleep(500);
if (!isRunning(pid) && !(await isHealthy(port, 1000))) return;
}
try {
process.kill(pid, "SIGKILL");
} catch {
// already stopped

if (!isRunning(pid)) {
throw new Error(
`Model Router PID ${pid} no longer reports as running after SIGTERM, but port ${port} remains healthy.`,
);
}
for (let _attempt = 0; _attempt < 5; _attempt++) {
await new Promise((resolve) => setTimeout(resolve, 500));
if (!isProcessRunning(pid) && !(await isRouterHealthy(port, 1000))) return;
if (
!doesModelRouterProcessOwnPort(pid, port, {
isRunning,
readCommandLine,
})
) {
throw new Error(
`Model Router ownership changed during shutdown for PID ${pid}; NemoClaw did not send an escalation signal.`,
);
}
throw new Error(
`Model Router shutdown did not converge after SIGTERM. NemoClaw refuses PID-based SIGKILL for PID ${pid} because process identity cannot be preserved atomically.`,
);
}

/**
Expand Down
30 changes: 30 additions & 0 deletions src/lib/onboard/runtime-control-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,4 +159,34 @@ describe("onboard runtime control flow", () => {
expect(effects).toEqual(["stop-router", "update-session"]);
expect(session.routerPid).toBeNull();
});

it("preserves durable session state when the Model Router stop fails", async () => {
const session = createSession({
agent: "langchain-deepagents-code",
provider: "nvidia",
routerPid: 1234,
});
const before = structuredClone(session);
const updateSession = vi.fn((mutator) => mutator(session) ?? session);
const plan = planSelectedAgentTransition(
{
resume: true,
session,
selectedAgentName: "openclaw",
routerPort: 4000,
note: () => undefined,
},
{
stopTrackedModelRouterForAgentChange: async () => {
throw new Error("Model Router stop failed");
},
updateSession,
},
);

await expect(plan.commit()).rejects.toThrow("Model Router stop failed");

expect(updateSession).not.toHaveBeenCalled();
expect(session).toEqual(before);
});
});
2 changes: 1 addition & 1 deletion test/managed-image-publication-workflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ describe("complete managed-image publication workflow", () => {
expect(qaBuilder.permissions).toEqual({ contents: "read" });
expect(qaBuilder.env).toMatchObject({
CANDIDATE_SHA: "${{ github.event.pull_request.head.sha }}",
STAGING_QA_SOURCE_SHA: "af2a73f0d6ce8f08a2975560f376470387c535d0",
STAGING_QA_SOURCE_SHA: "ce96811ddb418ad01c040521a1fe912b5bcb405e",
STAGING_QA_BASE_IMAGE: "nemoclaw-deepagents-code-base:staging-31396519688",
});
expect(qaBuilder.env).not.toHaveProperty("STAGING_PRODUCER_SHA");
Expand Down
Loading