diff --git a/docs/platforms/linux.md b/docs/platforms/linux.md index fc4b876b8f41..baccdcc0e05b 100644 --- a/docs/platforms/linux.md +++ b/docs/platforms/linux.md @@ -242,9 +242,9 @@ sessions and channel connections, so OpenClaw biases transient child processes to be killed first when possible. For eligible Linux child spawns, OpenClaw wraps the command in a short -`/bin/sh` shim that raises the child's own `oom_score_adj` to `1000`, then -`exec`s the real command. This is unprivileged: a process may always raise -its own OOM score. +`/bin/sh` shim that attempts to raise the child's own `oom_score_adj` to +`1000`, then `exec`s the real command. This is unprivileged: a process may +always raise its own OOM score. Covered child process surfaces: @@ -256,6 +256,9 @@ Covered child process surfaces: The wrapper is Linux-only and skipped when `/bin/sh` is unavailable, or when the child env sets `OPENCLAW_CHILD_OOM_SCORE_ADJ` to `0`, `false`, `no`, or `off`. +Use this opt-out only for controlled diagnosis: it removes child-first OOM +protection and makes the Gateway more likely to be selected as the victim under +real memory pressure. Verify a child process: @@ -263,8 +266,9 @@ Verify a child process: cat /proc//oom_score_adj ``` -Expected value for covered children is `1000`; the Gateway process itself -keeps its normal score (usually `0`). +When the write succeeds, the expected value for covered children is `1000`. +If `/proc` is unavailable or unwritable, the child still runs without the OOM +bias. The Gateway process itself keeps its normal score (usually `0`). The systemd unit's `OOMPolicy=continue` keeps the Gateway service alive when a transient child is selected by the OOM killer instead of marking the whole diff --git a/src/agents/bash-tools.exec-foreground-failures.test.ts b/src/agents/bash-tools.exec-foreground-failures.test.ts index e66c0cb49d67..a2013cd7aeca 100644 --- a/src/agents/bash-tools.exec-foreground-failures.test.ts +++ b/src/agents/bash-tools.exec-foreground-failures.test.ts @@ -9,10 +9,11 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createTempDirTracker } from "../../test/helpers/temp-dir.js"; import type { ProcessSupervisor } from "../process/supervisor/index.js"; -import type { SpawnInput } from "../process/supervisor/types.js"; +import type { RunExit, SpawnInput } from "../process/supervisor/types.js"; import { captureEnv } from "../test-utils/env.js"; import { resetProcessRegistryForTests } from "./bash-process-registry.test-support.js"; import { createExecTool } from "./bash-tools.exec-run.js"; +import { runExecProcess } from "./bash-tools.exec-runtime.js"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; import { getBashShellConfig } from "./shell-utils.js"; @@ -54,25 +55,21 @@ function requireFailedDetails( return details; } -function mockSuccessfulSpawn(stdout = "ok\n") { +function mockSpawn(exit: Partial = {}) { supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => ({ - runId: input.runId ?? "call-success", + runId: input.runId ?? "call", pid: 1234, startedAtMs: Date.now(), - stdin: { - write: vi.fn(), - end: vi.fn(), - destroy: vi.fn(), - }, wait: vi.fn(async () => ({ reason: "exit" as const, exitCode: 0, exitSignal: null, durationMs: 1, - stdout, + stdout: "", stderr: "", timedOut: false, noOutputTimedOut: false, + ...exit, })), cancel: vi.fn(), })); @@ -175,7 +172,7 @@ describe("exec foreground failures", () => { }); it("keeps the background fallback warning when gateway exec actually runs inline", async () => { - mockSuccessfulSpawn(); + mockSpawn(); const tool = createExecTool({ host: "gateway", security: "full", @@ -202,27 +199,13 @@ describe("exec foreground failures", () => { backgroundMs: 10, allowBackground: false, }); - supervisorMock.spawn.mockImplementationOnce(async (input: SpawnInput) => ({ - runId: input.runId ?? "call-timeout", - pid: 1234, - startedAtMs: Date.now(), - stdin: { - write: vi.fn(), - end: vi.fn(), - destroy: vi.fn(), - }, - wait: vi.fn(async () => ({ - reason: "overall-timeout" as const, - exitCode: null, - exitSignal: "SIGKILL" as NodeJS.Signals, - durationMs: input.timeoutMs ?? 50, - stdout: "", - stderr: "", - timedOut: true, - noOutputTimedOut: false, - })), - cancel: vi.fn(), - })); + mockSpawn({ + reason: "overall-timeout", + exitCode: null, + exitSignal: "SIGKILL", + oomScoreWrapperSelected: true, + timedOut: true, + }); const result = await tool.execute("call-timeout", { command: "echo never-runs", @@ -237,6 +220,8 @@ describe("exec foreground failures", () => { expect(text).toContain("Verify the resulting state before retrying"); expect(text).toContain("Do not automatically rerun non-idempotent commands"); expect(text).toContain("known to be safe to retry"); + expect(text).not.toContain("OOM-score wrapper"); + expect(text).not.toContain("OPENCLAW_CHILD_OOM_SCORE_ADJ"); const details = requireFailedDetails(result.details); expect(details.exitCode).toBeNull(); expect(details.exitSignal).toBe("SIGKILL"); @@ -249,6 +234,112 @@ describe("exec foreground failures", () => { expect(details.durationMs).toBeGreaterThanOrEqual(0); }); + it.each([ + { name: "child SIGKILL", pty: false, exitSignal: "SIGKILL" as NodeJS.Signals }, + { name: "PTY signal 9", pty: true, exitSignal: 9 }, + ])("adds cautious Linux OOM guidance for a wrapped $name", async ({ pty, exitSignal }) => { + mockSpawn({ + reason: "signal", + exitCode: pty ? 0 : null, + exitSignal, + oomScoreWrapperSelected: true, + }); + const tool = createExecTool({ + security: "full", + ask: "off", + allowBackground: false, + }); + + const result = await tool.execute(`call-oom-${exitSignal}`, { + command: "find . -type f", + host: "gateway", + pty, + }); + + expect(supervisorMock.spawn.mock.calls[0]?.[0]?.mode).toBe(pty ? "pty" : "child"); + const text = requireTextContent(result); + for (const fragment of [ + `Command aborted by signal ${exitSignal}`, + "OpenClaw selected its Linux OOM-score wrapper", + "attempts to set this child's oom_score_adj to 1000", + "SIGKILL alone does not identify whether the Linux OOM killer", + "Check cgroup memory events or kernel logs", + "If they show memory pressure, narrow the command", + "adjust memory, concurrency, or resource limits", + ]) { + expect(text).toContain(fragment); + } + expect(text).not.toContain("OPENCLAW_CHILD_OOM_SCORE_ADJ"); + }); + + it("keeps wrapped SIGKILL process outcomes generic for non-foreground consumers", async () => { + mockSpawn({ + reason: "signal", + exitCode: null, + exitSignal: "SIGKILL", + oomScoreWrapperSelected: true, + }); + + const run = await runExecProcess({ + command: "sleep 10", + workdir: process.cwd(), + env: {}, + usePty: false, + warnings: [], + maxOutput: 1_000, + pendingMaxOutput: 1_000, + notifyOnExit: false, + timeoutSec: null, + }); + + await expect(run.promise).resolves.toMatchObject({ + status: "failed", + reason: "Command aborted by signal SIGKILL", + oomScoreWrapperSelected: true, + }); + }); + + it.each([ + { + name: "unwrapped SIGKILL", + exitSignal: "SIGKILL" as NodeJS.Signals, + oomScoreWrapperSelected: false, + reason: "signal" as const, + }, + { + name: "wrapped non-SIGKILL signal", + exitSignal: "SIGTERM" as NodeJS.Signals, + oomScoreWrapperSelected: true, + reason: "signal" as const, + }, + { + name: "wrapped manual cancellation", + exitSignal: "SIGKILL" as NodeJS.Signals, + oomScoreWrapperSelected: true, + reason: "manual-cancel" as const, + }, + ])( + "preserves the generic signal message for $name", + async ({ exitSignal, oomScoreWrapperSelected, reason }) => { + mockSpawn({ reason, exitCode: null, exitSignal, oomScoreWrapperSelected }); + const tool = createExecTool({ + security: "full", + ask: "off", + allowBackground: false, + }); + + const result = await tool.execute(`call-generic-${reason}-${exitSignal}`, { + command: "sleep 10", + host: "gateway", + }); + + const text = requireTextContent(result); + expect(text).toContain(`Command aborted by signal ${exitSignal}`); + expect(text).not.toContain("OOM-score wrapper"); + expect(text).not.toContain("OPENCLAW_CHILD_OOM_SCORE_ADJ"); + }, + ); + it("rejects invalid host values before launching a command", async () => { const tool = createExecTool({ security: "full", @@ -351,7 +442,7 @@ describe("exec foreground failures", () => { it("defaults omitted sandbox workdirs to the sandbox workspace", async () => { const workspaceDir = tempDirs.make("openclaw-sandbox-workdir-"); - mockSuccessfulSpawn(); + mockSpawn(); const tool = createExecTool({ host: "sandbox", @@ -388,7 +479,7 @@ describe("exec foreground failures", () => { it("lets backend-validated sandbox workdirs reach the backend without host stat fallback", async () => { const workspaceDir = tempDirs.make("openclaw-sandbox-workdir-"); const { buildExecSpec, tool, validateWorkdir } = createBackendSandboxTool({ workspaceDir }); - mockSuccessfulSpawn(); + mockSpawn(); try { const result = await tool.execute("call-remote-sandbox-workdir", { @@ -472,7 +563,7 @@ describe("exec foreground failures", () => { const workspaceDir = tempDirs.make("openclaw-sandbox-workdir-"); fs.writeFileSync(path.join(workspaceDir, "script.py"), "print($TOKEN)\n"); const { buildExecSpec, tool, validateWorkdir } = createBackendSandboxTool({ workspaceDir }); - mockSuccessfulSpawn(); + mockSpawn(); try { const result = await tool.execute("call-remote-only-script", { @@ -495,7 +586,7 @@ describe("exec foreground failures", () => { const srcDir = path.join(workspaceDir, "src"); fs.mkdirSync(srcDir); const { buildExecSpec, tool, validateWorkdir } = createBackendSandboxTool({ workspaceDir }); - mockSuccessfulSpawn(); + mockSpawn(); try { const result = await tool.execute("call-relative-remote-sandbox-workdir", { diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index 80bd2cb8ad4b..e8d147a3f717 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -145,6 +145,7 @@ export type ExecProcessOutcome = timedOut: boolean; noOutputTimedOut?: boolean; failureKind: ExecProcessFailureKind; + oomScoreWrapperSelected?: boolean; reason: string; }; @@ -519,6 +520,7 @@ function buildExecExitOutcome(params: { timedOut: params.exit.timedOut, noOutputTimedOut: params.exit.noOutputTimedOut, failureKind, + oomScoreWrapperSelected: params.exit.oomScoreWrapperSelected, reason: joinExecFailureOutput(params.aggregated, reason), }; } diff --git a/src/agents/bash-tools.exec-support.ts b/src/agents/bash-tools.exec-support.ts index 1de59e4a5769..98ed28fd5769 100644 --- a/src/agents/bash-tools.exec-support.ts +++ b/src/agents/bash-tools.exec-support.ts @@ -16,7 +16,16 @@ export function buildExecForegroundResult(params: { }): AgentToolResult { const warningText = params.warningText?.trim() ? `${params.warningText}\n\n` : ""; if (params.outcome.status === "failed") { - return failedTextResult(`${warningText}${params.outcome.reason}`, { + const linuxOomGuidance = + params.outcome.failureKind === "signal" && + params.outcome.exitReason === "signal" && + params.outcome.oomScoreWrapperSelected === true && + (params.outcome.exitSignal === "SIGKILL" || params.outcome.exitSignal === 9) + ? "\n\nOpenClaw selected its Linux OOM-score wrapper, which attempts to set this child's oom_score_adj to 1000. " + + "SIGKILL alone does not identify whether the Linux OOM killer, an operator, or another process sent it. " + + "Check cgroup memory events or kernel logs. If they show memory pressure, narrow the command or adjust memory, concurrency, or resource limits." + : ""; + return failedTextResult(`${warningText}${params.outcome.reason}${linuxOomGuidance}`, { status: "failed", exitCode: params.outcome.exitCode ?? null, exitSignal: params.outcome.exitSignal, diff --git a/src/process/supervisor/adapters/child.test.ts b/src/process/supervisor/adapters/child.test.ts index fabcabe9cebb..b7cdea00f0ae 100644 --- a/src/process/supervisor/adapters/child.test.ts +++ b/src/process/supervisor/adapters/child.test.ts @@ -746,10 +746,11 @@ describe("createChildAdapter", () => { process.env.ENV = "/tmp/env"; process.env.CDPATH = "/tmp"; try { - await createAdapterHarness({ + const { adapter } = await createAdapterHarness({ pid: 3334, argv: ["/usr/bin/node", "-e", "process.exit(0)"], }); + expect(adapter.oomScoreWrapperSelected).toBe(true); } finally { if (originalBashEnv === undefined) { delete process.env.BASH_ENV; diff --git a/src/process/supervisor/adapters/child.ts b/src/process/supervisor/adapters/child.ts index 309e3833788f..7e08e4b18e98 100644 --- a/src/process/supervisor/adapters/child.ts +++ b/src/process/supervisor/adapters/child.ts @@ -495,6 +495,7 @@ export async function createChildAdapter(params: { return { pid: child.pid ?? undefined, stdin, + oomScoreWrapperSelected: preparedSpawn.wrapped, onStdout, onStderr, wait, diff --git a/src/process/supervisor/adapters/pty.test.ts b/src/process/supervisor/adapters/pty.test.ts index 08dd380c71ad..ec16bc7898cf 100644 --- a/src/process/supervisor/adapters/pty.test.ts +++ b/src/process/supervisor/adapters/pty.test.ts @@ -320,11 +320,12 @@ describe("createPtyAdapter", () => { const stub = createStubPty(); spawnMock.mockReturnValue(stub); - await createPtyAdapter({ + const adapter = await createPtyAdapter({ shell: "bash", args: ["-lc", "env"], env: { PATH: "/usr/bin", BASH_ENV: "/tmp/bashenv", TERM: "dumb" }, }); + expect(adapter.oomScoreWrapperSelected).toBe(true); } finally { if (originalPlatform) { Object.defineProperty(process, "platform", originalPlatform); diff --git a/src/process/supervisor/adapters/pty.ts b/src/process/supervisor/adapters/pty.ts index 8361b6705083..6b9005f04afd 100644 --- a/src/process/supervisor/adapters/pty.ts +++ b/src/process/supervisor/adapters/pty.ts @@ -207,6 +207,7 @@ export async function createPtyAdapter(params: { return { pid: pty.pid || undefined, stdin, + oomScoreWrapperSelected: preparedSpawn.wrapped, onStdout, onStderr, wait, diff --git a/src/process/supervisor/supervisor.test.ts b/src/process/supervisor/supervisor.test.ts index 14e46e49944b..4ab6243c5eb4 100644 --- a/src/process/supervisor/supervisor.test.ts +++ b/src/process/supervisor/supervisor.test.ts @@ -121,6 +121,7 @@ describe("process supervisor", () => { it("spawns child runs and captures output", async () => { const adapter = createStubChildAdapter(); + adapter.oomScoreWrapperSelected = true; createChildAdapterMock.mockResolvedValue(adapter); const supervisor = createProcessSupervisor(); @@ -138,6 +139,7 @@ describe("process supervisor", () => { expect(exit.reason).toBe("exit"); expect(exit.exitCode).toBe(0); expect(exit.stdout).toBe("ok"); + expect(exit.oomScoreWrapperSelected).toBe(true); expect(adapter.disposeMock).toHaveBeenCalledTimes(1); }); diff --git a/src/process/supervisor/supervisor.ts b/src/process/supervisor/supervisor.ts index a545f7fd35ab..59a2d6122b5f 100644 --- a/src/process/supervisor/supervisor.ts +++ b/src/process/supervisor/supervisor.ts @@ -393,6 +393,7 @@ export function createProcessSupervisor(): ProcessSupervisor { reason, exitCode: result.code, exitSignal: result.signal, + oomScoreWrapperSelected: adapter.oomScoreWrapperSelected === true, durationMs: Date.now() - startedAtMs, stdout, stderr, diff --git a/src/process/supervisor/types.ts b/src/process/supervisor/types.ts index 448a674bb990..d1459c8f11da 100644 --- a/src/process/supervisor/types.ts +++ b/src/process/supervisor/types.ts @@ -30,6 +30,7 @@ export type RunExit = { reason: TerminationReason; exitCode: number | null; exitSignal: NodeJS.Signals | number | null; + oomScoreWrapperSelected?: boolean; durationMs: number; stdout: string; stderr: string; @@ -66,6 +67,7 @@ export type SpawnSecretInput = { export type SpawnProcessAdapter = { pid?: number; stdin?: ManagedRunStdin; + oomScoreWrapperSelected?: boolean; onStdout: (listener: (chunk: string) => void) => void; onStderr: (listener: (chunk: string) => void) => void; wait: () => Promise<{ code: number | null; signal: WaitSignal }>;