fix(exec): isolate completed-process retention

This commit is contained in:
Amp
2026-08-21 15:04:13 +00:00
parent 9050d9b0a7
commit c028581cce
11 changed files with 112 additions and 44 deletions
@@ -4,7 +4,7 @@
*/
import fs from "node:fs";
import path from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import "./test-helpers/fast-coding-tools.js";
import "./test-helpers/fast-openclaw-tools.js";
import { createTempDirTracker } from "../../test/helpers/temp-dir.js";
@@ -12,6 +12,8 @@ import type { OpenClawConfig } from "../config/config.js";
import { setActivePluginRegistry } from "../plugins/runtime.js";
import { createSessionConversationTestRegistry } from "../test-utils/session-conversation-registry.js";
import { createOpenClawCodingTools } from "./agent-tools.js";
import { getFinishedSession } from "./bash-process-registry.js";
import { resetProcessRegistryForTests } from "./bash-process-registry.test-support.js";
import { resolveExecToolConfig } from "./lazy-exec-tool.js";
function createExecHostDefaultsConfig(
@@ -71,6 +73,7 @@ describe("Agent-specific exec tool defaults", () => {
});
afterEach(() => {
resetProcessRegistryForTests();
tempDirs.cleanup();
});
@@ -381,4 +384,44 @@ describe("Agent-specific exec tool defaults", () => {
const details = result?.details as { status?: string } | undefined;
expect(details?.status).toBe("completed");
});
it("admits per-agent cleanup retention with the background exec", async () => {
const cleanupMs = 3 * 60 * 60 * 1000;
const tools = createOpenClawCodingTools({
config: {
tools: {
exec: {
host: "gateway",
mode: "full",
cleanupMs: 60 * 1000,
},
},
agents: {
list: [{ id: "main", tools: { exec: { cleanupMs } } }],
},
},
exec: { backgroundMs: 0 },
sessionKey: "agent:main:main",
...createTempAgentDirs("test-main-cleanup-retention"),
});
const result = await requireExecTool(tools).execute("call-main-cleanup-retention", {
command: "echo done",
background: true,
});
const sessionId = (result.details as { sessionId?: string }).sessionId;
expect(sessionId).toEqual(expect.any(String));
if (!sessionId) {
throw new Error("expected a background process session");
}
await vi.waitFor(() => {
expect(getFinishedSession(sessionId)).toBeDefined();
});
const finished = getFinishedSession(sessionId);
if (!finished) {
throw new Error("expected a finished process session");
}
expect(finished.expiresAt - finished.endedAt).toBe(cleanupMs);
});
});
+2 -2
View File
@@ -561,7 +561,7 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions)
const imageSanitization = resolveImageSanitizationLimits(options?.config);
options?.recordToolPrepStage?.("workspace-policy");
const { cleanupMs: cleanupMsOverride, ...execDefaults } = options?.exec ?? {};
const execDefaults = options?.exec ?? {};
const effectiveExecPolicy = applyExecPolicyLayer(execConfig, options?.exec);
const processToolAvailabilityRef: NonNullable<ExecToolDefaults["processToolAvailabilityRef"]> =
{};
@@ -607,6 +607,7 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions)
safeBinTrustedDirs: options?.exec?.safeBinTrustedDirs ?? execConfig.safeBinTrustedDirs,
safeBinProfiles: options?.exec?.safeBinProfiles ?? execConfig.safeBinProfiles,
agentId,
cleanupMs: options?.exec?.cleanupMs ?? execConfig.cleanupMs,
processToolAvailabilityRef,
scopeKey,
sessionKey: options?.sessionKey,
@@ -640,7 +641,6 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions)
options?.exec?.notifyOnExitEmptySuccess ?? execConfig.notifyOnExitEmptySuccess,
},
processDefaults: {
cleanupMs: cleanupMsOverride ?? execConfig.cleanupMs,
scopeKey,
},
recordToolPrepStage: options?.recordToolPrepStage,
@@ -3,7 +3,7 @@
* Provides complete session objects so tests can focus on the field under
* inspection without repeating registry defaults.
*/
import type { ProcessSession } from "./bash-process-registry.js";
import { type ProcessSession, resolveProcessCleanupMs } from "./bash-process-registry.js";
/** Build a process-session fixture with safe defaults for registry tests. */
export function createProcessSessionFixture(params: {
@@ -13,6 +13,7 @@ export function createProcessSessionFixture(params: {
cwd?: string;
maxOutputChars?: number;
pendingMaxOutputChars?: number;
cleanupMs?: number;
backgrounded?: boolean;
pid?: number;
cursorKeyMode?: ProcessSession["cursorKeyMode"];
@@ -20,6 +21,7 @@ export function createProcessSessionFixture(params: {
const session: ProcessSession = {
id: params.id,
command: params.command ?? "test",
cleanupMs: resolveProcessCleanupMs(params.cleanupMs),
startedAt: params.startedAt ?? Date.now(),
cwd: params.cwd ?? "/tmp",
maxOutputChars: params.maxOutputChars ?? 10_000,
+24 -14
View File
@@ -22,7 +22,6 @@ import {
markExited,
markTerminalPollObserved,
recordNotifyOnExitRemoval,
setJobTtlMs,
tail,
} from "./bash-process-registry.js";
import { createProcessSessionFixture } from "./bash-process-registry.test-helpers.js";
@@ -43,6 +42,7 @@ describe("bash process registry", () => {
maxOutputChars: number;
pendingMaxOutputChars: number;
backgrounded: boolean;
cleanupMs?: number;
}): ProcessSession {
return createProcessSessionFixture({
id: params.id ?? "sess",
@@ -50,6 +50,7 @@ describe("bash process registry", () => {
maxOutputChars: params.maxOutputChars,
pendingMaxOutputChars: params.pendingMaxOutputChars,
backgrounded: params.backgrounded,
cleanupMs: params.cleanupMs,
});
}
@@ -212,8 +213,8 @@ describe("bash process registry", () => {
markBackgrounded(session);
markExited(session, 0, null, "completed");
const finishedSessions = listFinishedSessions();
const endedAt = finishedSessions[0]?.endedAt;
expect(endedAt).toEqual(expect.any(Number));
const endedAt = finishedSessions[0]?.endedAt ?? 0;
expect(endedAt).toBeGreaterThan(0);
expect(finishedSessions).toStrictEqual([
{
id: "sess",
@@ -221,6 +222,7 @@ describe("bash process registry", () => {
scopeKey: undefined,
startedAt: session.startedAt,
endedAt,
expiresAt: endedAt + session.cleanupMs,
cwd: "/tmp",
status: "completed",
exitCode: 0,
@@ -411,29 +413,37 @@ describe("bash process registry", () => {
expect(getActiveBackgroundExecSessionCount()).toBe(0);
});
it("clamps a zero retention TTL to one minute", () => {
it("clamps and isolates finished-session retention at admission", () => {
vi.useFakeTimers();
try {
vi.setSystemTime(new Date("2026-07-09T00:00:00Z"));
setJobTtlMs(0);
const session = createRegistrySession({
id: "zero-ttl",
const shortRetention = createRegistrySession({
id: "short-retention",
maxOutputChars: 100,
pendingMaxOutputChars: 30_000,
backgrounded: true,
cleanupMs: 0,
});
addSession(session);
markExited(session, 0, null, "completed");
const longRetention = createRegistrySession({
id: "long-retention",
maxOutputChars: 100,
pendingMaxOutputChars: 30_000,
backgrounded: true,
cleanupMs: 3 * 60 * 60 * 1000,
});
addSession(shortRetention);
addSession(longRetention);
markExited(shortRetention, 0, null, "completed");
markExited(longRetention, 0, null, "completed");
vi.advanceTimersByTime(30_000);
expect(listFinishedSessions()).toHaveLength(1);
vi.advanceTimersByTime(60 * 1000);
expect(getFinishedSession(shortRetention.id)).toBeUndefined();
expect(getFinishedSession(longRetention.id)).toBeDefined();
vi.advanceTimersByTime(60_000);
vi.advanceTimersByTime(3 * 60 * 60 * 1000 - 60 * 1000);
expect(listFinishedSessions()).toHaveLength(0);
} finally {
resetProcessRegistryForTests();
setJobTtlMs(30 * 60 * 1000);
resetProcessRegistryForTests();
vi.useRealTimers();
}
+28 -20
View File
@@ -24,7 +24,12 @@ function clampTtl(value: number | undefined) {
return Math.min(Math.max(value, MIN_JOB_TTL_MS), MAX_JOB_TTL_MS);
}
let jobTtlMs = clampTtl(readEnvInt("OPENCLAW_BASH_JOB_TTL_MS", "PI_BASH_JOB_TTL_MS"));
const defaultJobTtlMs = clampTtl(readEnvInt("OPENCLAW_BASH_JOB_TTL_MS", "PI_BASH_JOB_TTL_MS"));
/** Resolves the immutable retention duration owned by one admitted exec process. */
export function resolveProcessCleanupMs(value?: number): number {
return value === undefined ? defaultJobTtlMs : clampTtl(value);
}
/** Lifecycle status recorded for background process sessions. */
type ProcessStatus = "running" | "completed" | "failed" | "killed";
@@ -55,6 +60,8 @@ export interface ProcessSession {
command: string;
scopeKey?: string;
sessionKey?: string;
/** Finished-result retention resolved when this exact exec process starts. */
cleanupMs: number;
/** Agent owner frozen when the exec process starts. */
agentId?: string;
/** `session.mainKey` from the runtime config, snapshotted at exec start.
@@ -117,6 +124,7 @@ interface FinishedSession {
scopeKey?: string;
startedAt: number;
endedAt: number;
expiresAt: number;
cwd?: string;
status: ProcessStatus;
exitCode?: number | null;
@@ -150,11 +158,10 @@ export function isProcessSessionIdTaken(id: string): boolean {
);
}
/** Adds a running session and starts retention sweeping if needed. */
/** Adds a running session with its admission-time retention already resolved. */
export function addSession(session: ProcessSession) {
processSessionStartOrders.set(session, nextProcessSessionStartOrder++);
runningSessions.set(session.id, session);
startSweeper();
}
/** Sorts registered process records newest-first, including same-millisecond starts. */
@@ -197,6 +204,7 @@ function deleteFinishedSession(id: string): boolean {
export function deleteSession(id: string) {
runningSessions.delete(id);
deleteFinishedSession(id);
scheduleSweeper();
}
/** Removes completed process records belonging to retired session identities. */
@@ -216,6 +224,7 @@ export function clearFinishedSessionsForScopes(scopeKeys: Iterable<string>): voi
deleteFinishedSession(id);
}
}
scheduleSweeper();
}
/** Appends process output while enforcing aggregate and pending-output caps. */
@@ -361,12 +370,14 @@ function moveToFinished(session: ProcessSession, status: ProcessStatus) {
// Keep full completed logs; evict older records rather than silently
// truncating the process poll/log contract or dropping the newest result.
deleteFinishedSession(session.id);
const endedAt = Date.now();
const finished: FinishedSession = {
id: session.id,
command: session.command,
scopeKey: session.scopeKey,
startedAt: session.startedAt,
endedAt: Date.now(),
endedAt,
expiresAt: endedAt + session.cleanupMs,
cwd: session.cwd,
status,
exitCode: session.exitCode,
@@ -397,6 +408,7 @@ function moveToFinished(session: ProcessSession, status: ProcessStatus) {
}
deleteFinishedSession(oldestSessionId);
}
scheduleSweeper();
}
/** Returns the last `max` characters of text without adding ellipses. */
@@ -468,30 +480,26 @@ if (process.env.VITEST || process.env.NODE_ENV === "test") {
{ resetProcessRegistryForTests };
}
/** Overrides finished-session retention TTL, clamped to supported bounds. */
export function setJobTtlMs(value?: number) {
if (value === undefined || Number.isNaN(value)) {
return;
}
jobTtlMs = clampTtl(value);
stopSweeper();
startSweeper();
}
function pruneFinishedSessions() {
const cutoff = Date.now() - jobTtlMs;
const now = Date.now();
for (const [id, session] of finishedSessions.entries()) {
if (session.endedAt < cutoff) {
if (session.expiresAt <= now) {
deleteFinishedSession(id);
}
}
scheduleSweeper();
}
function startSweeper() {
if (sweeper) {
function scheduleSweeper() {
stopSweeper();
let nextExpiration = Number.POSITIVE_INFINITY;
for (const session of finishedSessions.values()) {
nextExpiration = Math.min(nextExpiration, session.expiresAt);
}
if (!Number.isFinite(nextExpiration)) {
return;
}
sweeper = setInterval(pruneFinishedSessions, Math.max(30_000, jobTtlMs / 6));
sweeper = setTimeout(pruneFinishedSessions, Math.max(0, nextExpiration - Date.now()));
sweeper.unref?.();
}
@@ -499,6 +507,6 @@ function stopSweeper() {
if (!sweeper) {
return;
}
clearInterval(sweeper);
clearTimeout(sweeper);
sweeper = null;
}
@@ -127,6 +127,7 @@ type ProcessGatewayAllowlistParams = {
approvalRunningNoticeMs: number;
maxOutput: number;
pendingMaxOutput: number;
cleanupMs?: number;
processContinuationAvailable?: boolean;
trustedSafeBinDirs?: ReadonlySet<string>;
};
@@ -1393,6 +1394,7 @@ export async function processGatewayAllowlist(
warnings: params.warnings,
maxOutput: params.maxOutput,
pendingMaxOutput: params.pendingMaxOutput,
cleanupMs: params.cleanupMs,
notifyOnExit: false,
notifyOnExitEmptySuccess: false,
scopeKey: params.scopeKey,
+2
View File
@@ -523,6 +523,7 @@ export function createExecTool(
approvalRunningNoticeMs,
maxOutput,
pendingMaxOutput,
cleanupMs: defaults?.cleanupMs,
processContinuationAvailable: allowBackground,
trustedSafeBinDirs,
});
@@ -574,6 +575,7 @@ export function createExecTool(
warnings,
maxOutput,
pendingMaxOutput,
cleanupMs: defaults?.cleanupMs,
notifyOnExit,
notifyOnExitEmptySuccess,
scopeKey: defaults?.scopeKey,
+3
View File
@@ -43,6 +43,7 @@ import {
isProcessSessionIdTaken,
markExited,
recordNotifyOnExitRemoval,
resolveProcessCleanupMs,
tail,
} from "./bash-process-registry.js";
import {
@@ -634,6 +635,7 @@ export async function runExecProcess(opts: {
warnings: string[];
maxOutput: number;
pendingMaxOutput: number;
cleanupMs?: number;
notifyOnExit: boolean;
notifyOnExitEmptySuccess?: boolean;
scopeKey?: string;
@@ -673,6 +675,7 @@ export async function runExecProcess(opts: {
command: opts.command,
scopeKey: opts.scopeKey,
sessionKey: opts.sessionKey,
cleanupMs: resolveProcessCleanupMs(opts.cleanupMs),
agentId: opts.agentId,
mainKey: opts.mainKey,
sessionScope: opts.sessionScope,
+1
View File
@@ -48,6 +48,7 @@ export type ExecToolDefaults = {
autoReviewer?: ExecAutoReviewer;
agentId?: string;
backgroundMs?: number;
cleanupMs?: number;
timeoutSec?: number;
approvalWarningText?: string;
approvalFollowupText?: string;
+1 -6
View File
@@ -21,7 +21,6 @@ import {
listFinishedSessions,
listRunningSessions,
markTerminalPollObserved,
setJobTtlMs,
} from "./bash-process-registry.js";
import { describeProcessTool } from "./bash-tools.descriptions.js";
import { appendExecTimeoutRetryGuidance, renderExecExitLabel } from "./bash-tools.exec-output.js";
@@ -47,7 +46,6 @@ import type { AgentToolWithMeta } from "./tools/common.js";
/** Defaults injected by tests, agent scopes, and scoped process registries. */
export type ProcessToolDefaults = {
cleanupMs?: number;
hasCronTool?: boolean;
inputWaitIdleMs?: number;
scopeKey?: string;
@@ -249,13 +247,10 @@ async function sleepPollInterval(ms: number, signal?: AbortSignal): Promise<void
});
}
/** Build the process-control tool with optional cleanup, scope, and input-idle defaults. */
/** Build the process-control tool with optional scope and input-idle defaults. */
export function createProcessTool(
defaults?: ProcessToolDefaults,
): AgentToolWithMeta<typeof processSchema, unknown> {
if (defaults?.cleanupMs !== undefined) {
setJobTtlMs(defaults.cleanupMs);
}
const scopeKey = defaults?.scopeKey;
const supervisor = getProcessSupervisor();
const inputWaitIdleMs = clampWithDefault(
+2
View File
@@ -19,6 +19,7 @@ import {
markBackgrounded,
markExited,
type ProcessSession,
resolveProcessCleanupMs,
} from "./bash-process-registry.js";
import { resetProcessRegistryForTests } from "./bash-process-registry.test-support.js";
import { createExecTool, createProcessTool } from "./bash-tools.js";
@@ -636,6 +637,7 @@ const seedFinishedLogSession = (lines: string[]) => {
const session: ProcessSession = {
id: `seeded-log-${nextCallId()}`,
command: "seeded log",
cleanupMs: resolveProcessCleanupMs(),
startedAt: Date.now(),
maxOutputChars: 100_000,
pendingMaxOutputChars: 100_000,