fix: clean agent bash lint

This commit is contained in:
Peter Steinberger
2025-12-25 03:29:36 +01:00
parent 2442186a31
commit 92f467e81c
5 changed files with 90 additions and 48 deletions

View File

@@ -1,4 +1,6 @@
import type { ChildProcessWithoutNullStreams } from "node:child_process";
import { beforeEach, describe, expect, it } from "vitest"; import { beforeEach, describe, expect, it } from "vitest";
import type { ProcessSession } from "./bash-process-registry.js";
import { import {
addSession, addSession,
appendOutput, appendOutput,
@@ -8,20 +10,16 @@ import {
resetProcessRegistryForTests, resetProcessRegistryForTests,
} from "./bash-process-registry.js"; } from "./bash-process-registry.js";
type DummyChild = {
pid?: number;
};
describe("bash process registry", () => { describe("bash process registry", () => {
beforeEach(() => { beforeEach(() => {
resetProcessRegistryForTests(); resetProcessRegistryForTests();
}); });
it("captures output and truncates", () => { it("captures output and truncates", () => {
const session = { const session: ProcessSession = {
id: "sess", id: "sess",
command: "echo test", command: "echo test",
child: { pid: 123 } as DummyChild, child: { pid: 123 } as ChildProcessWithoutNullStreams,
startedAt: Date.now(), startedAt: Date.now(),
cwd: "/tmp", cwd: "/tmp",
maxOutputChars: 10, maxOutputChars: 10,
@@ -37,19 +35,19 @@ describe("bash process registry", () => {
backgrounded: false, backgrounded: false,
}; };
addSession(session as any); addSession(session);
appendOutput(session as any, "stdout", "0123456789"); appendOutput(session, "stdout", "0123456789");
appendOutput(session as any, "stdout", "abcdef"); appendOutput(session, "stdout", "abcdef");
expect(session.aggregated).toBe("6789abcdef"); expect(session.aggregated).toBe("6789abcdef");
expect(session.truncated).toBe(true); expect(session.truncated).toBe(true);
}); });
it("only persists finished sessions when backgrounded", () => { it("only persists finished sessions when backgrounded", () => {
const session = { const session: ProcessSession = {
id: "sess", id: "sess",
command: "echo test", command: "echo test",
child: { pid: 123 } as DummyChild, child: { pid: 123 } as ChildProcessWithoutNullStreams,
startedAt: Date.now(), startedAt: Date.now(),
cwd: "/tmp", cwd: "/tmp",
maxOutputChars: 100, maxOutputChars: 100,
@@ -65,12 +63,12 @@ describe("bash process registry", () => {
backgrounded: false, backgrounded: false,
}; };
addSession(session as any); addSession(session);
markExited(session as any, 0, null, "completed"); markExited(session, 0, null, "completed");
expect(listFinishedSessions()).toHaveLength(0); expect(listFinishedSessions()).toHaveLength(0);
markBackgrounded(session as any); markBackgrounded(session);
markExited(session as any, 0, null, "completed"); markExited(session, 0, null, "completed");
expect(listFinishedSessions()).toHaveLength(1); expect(listFinishedSessions()).toHaveLength(1);
}); });
}); });

View File

@@ -79,12 +79,17 @@ export function appendOutput(
) { ) {
session.pendingStdout ??= []; session.pendingStdout ??= [];
session.pendingStderr ??= []; session.pendingStderr ??= [];
const buffer = stream === "stdout" ? session.pendingStdout : session.pendingStderr; const buffer =
stream === "stdout" ? session.pendingStdout : session.pendingStderr;
buffer.push(chunk); buffer.push(chunk);
session.totalOutputChars += chunk.length; session.totalOutputChars += chunk.length;
const aggregated = trimWithCap(session.aggregated + chunk, session.maxOutputChars); const aggregated = trimWithCap(
session.aggregated + chunk,
session.maxOutputChars,
);
session.truncated = session.truncated =
session.truncated || aggregated.length < session.aggregated.length + chunk.length; session.truncated ||
aggregated.length < session.aggregated.length + chunk.length;
session.aggregated = aggregated; session.aggregated = aggregated;
session.tail = tail(session.aggregated, 2000); session.tail = tail(session.aggregated, 2000);
} }
@@ -175,6 +180,9 @@ function pruneFinishedSessions() {
function startSweeper() { function startSweeper() {
if (sweeper) return; if (sweeper) return;
sweeper = setInterval(pruneFinishedSessions, Math.max(30_000, JOB_TTL_MS / 6)); sweeper = setInterval(
pruneFinishedSessions,
Math.max(30_000, JOB_TTL_MS / 6),
);
sweeper.unref?.(); sweeper.unref?.();
} }

View File

@@ -1,6 +1,6 @@
import { beforeEach, describe, expect, it } from "vitest"; import { beforeEach, describe, expect, it } from "vitest";
import { bashTool, processTool } from "./bash-tools.js";
import { resetProcessRegistryForTests } from "./bash-process-registry.js"; import { resetProcessRegistryForTests } from "./bash-process-registry.js";
import { bashTool, processTool } from "./bash-tools.js";
const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
@@ -49,7 +49,9 @@ describe("bash tool backgrounding", () => {
const sessionId = (result.details as { sessionId: string }).sessionId; const sessionId = (result.details as { sessionId: string }).sessionId;
const list = await processTool.execute("call2", { action: "list" }); const list = await processTool.execute("call2", { action: "list" });
const sessions = (list.details as { sessions: Array<{ sessionId: string }> }).sessions; const sessions = (
list.details as { sessions: Array<{ sessionId: string }> }
).sessions;
expect(sessions.some((s) => s.sessionId === sessionId)).toBe(true); expect(sessions.some((s) => s.sessionId === sessionId)).toBe(true);
}); });
}); });

View File

@@ -1,8 +1,8 @@
import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process";
import { randomUUID } from "node:crypto";
import type { AgentTool, AgentToolResult } from "@mariozechner/pi-ai"; import type { AgentTool, AgentToolResult } from "@mariozechner/pi-ai";
import { StringEnum } from "@mariozechner/pi-ai"; import { StringEnum } from "@mariozechner/pi-ai";
import { Type } from "@sinclair/typebox"; import { Type } from "@sinclair/typebox";
import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process";
import { randomUUID } from "node:crypto";
import { import {
addSession, addSession,
@@ -16,7 +16,11 @@ import {
markBackgrounded, markBackgrounded,
markExited, markExited,
} from "./bash-process-registry.js"; } from "./bash-process-registry.js";
import { getShellConfig, killProcessTree, sanitizeBinaryOutput } from "./shell-utils.js"; import {
getShellConfig,
killProcessTree,
sanitizeBinaryOutput,
} from "./shell-utils.js";
const CHUNK_LIMIT = 8 * 1024; const CHUNK_LIMIT = 8 * 1024;
const DEFAULT_YIELD_MS = clampNumber( const DEFAULT_YIELD_MS = clampNumber(
@@ -79,7 +83,7 @@ export const bashTool: AgentTool<typeof bashSchema, BashToolDetails> = {
description: description:
"Execute bash with background continuation. Use yieldMs/background to continue later via process tool.", "Execute bash with background continuation. Use yieldMs/background to continue later via process tool.",
parameters: bashSchema, parameters: bashSchema,
execute: async (toolCallId, args, signal, onUpdate) => { execute: async (_toolCallId, args, signal, onUpdate) => {
const params = args as { const params = args as {
command: string; command: string;
workdir?: string; workdir?: string;
@@ -106,12 +110,13 @@ export const bashTool: AgentTool<typeof bashSchema, BashToolDetails> = {
const workdir = params.workdir?.trim() || process.cwd(); const workdir = params.workdir?.trim() || process.cwd();
const { shell, args: shellArgs } = getShellConfig(); const { shell, args: shellArgs } = getShellConfig();
const env = params.env ?? {};
const child: ChildProcessWithoutNullStreams = spawn( const child: ChildProcessWithoutNullStreams = spawn(
shell, shell,
[...shellArgs, params.command], [...shellArgs, params.command],
{ {
cwd: workdir, cwd: workdir,
env: { ...process.env, ...(params.env ?? {}) }, env: { ...process.env, ...env },
detached: true, detached: true,
stdio: ["pipe", "pipe", "pipe"], stdio: ["pipe", "pipe", "pipe"],
}, },
@@ -243,8 +248,11 @@ export const bashTool: AgentTool<typeof bashSchema, BashToolDetails> = {
if (timeoutTimer) clearTimeout(timeoutTimer); if (timeoutTimer) clearTimeout(timeoutTimer);
const durationMs = Date.now() - startedAt; const durationMs = Date.now() - startedAt;
const wasSignal = exitSignal != null; const wasSignal = exitSignal != null;
const isSuccess = code === 0 && !wasSignal && !signal?.aborted && !timedOut; const isSuccess =
const status: "completed" | "failed" = isSuccess ? "completed" : "failed"; code === 0 && !wasSignal && !signal?.aborted && !timedOut;
const status: "completed" | "failed" = isSuccess
? "completed"
: "failed";
markExited(session, code, exitSignal, status); markExited(session, code, exitSignal, status);
if (yielded || session.backgrounded) return; if (yielded || session.backgrounded) return;
@@ -352,7 +360,10 @@ export const processTool: AgentTool<typeof processSchema> = {
); );
return { return {
content: [ content: [
{ type: "text", text: lines.join("\n") || "No running or recent sessions." }, {
type: "text",
text: lines.join("\n") || "No running or recent sessions.",
},
], ],
details: { status: "completed", sessions: [...running, ...finished] }, details: { status: "completed", sessions: [...running, ...finished] },
}; };
@@ -360,7 +371,9 @@ export const processTool: AgentTool<typeof processSchema> = {
if (!params.sessionId) { if (!params.sessionId) {
return { return {
content: [{ type: "text", text: "sessionId is required for this action." }], content: [
{ type: "text", text: "sessionId is required for this action." },
],
details: { status: "failed" }, details: { status: "failed" },
}; };
} }
@@ -389,7 +402,8 @@ export const processTool: AgentTool<typeof processSchema> = {
}, },
], ],
details: { details: {
status: finished.status === "completed" ? "completed" : "failed", status:
finished.status === "completed" ? "completed" : "failed",
sessionId: params.sessionId, sessionId: params.sessionId,
exitCode: finished.exitCode ?? undefined, exitCode: finished.exitCode ?? undefined,
aggregated: finished.aggregated, aggregated: finished.aggregated,
@@ -398,7 +412,10 @@ export const processTool: AgentTool<typeof processSchema> = {
} }
return { return {
content: [ content: [
{ type: "text", text: `No session found for ${params.sessionId}` }, {
type: "text",
text: `No session found for ${params.sessionId}`,
},
], ],
details: { status: "failed" }, details: { status: "failed" },
}; };
@@ -421,7 +438,12 @@ export const processTool: AgentTool<typeof processSchema> = {
if (exited) { if (exited) {
const status = const status =
exitCode === 0 && exitSignal == null ? "completed" : "failed"; exitCode === 0 && exitSignal == null ? "completed" : "failed";
markExited(session, session.exitCode ?? null, session.exitSignal ?? null, status); markExited(
session,
session.exitCode ?? null,
session.exitSignal ?? null,
status,
);
} }
const status = exited const status = exited
? exitCode === 0 && exitSignal == null ? exitCode === 0 && exitSignal == null
@@ -470,9 +492,7 @@ export const processTool: AgentTool<typeof processSchema> = {
const total = session.aggregated.length; const total = session.aggregated.length;
const slice = session.aggregated.slice( const slice = session.aggregated.slice(
params.offset ?? 0, params.offset ?? 0,
params.limit params.limit ? (params.offset ?? 0) + params.limit : undefined,
? (params.offset ?? 0) + params.limit
: undefined,
); );
return { return {
content: [{ type: "text", text: slice || "(no output yet)" }], content: [{ type: "text", text: slice || "(no output yet)" }],
@@ -488,15 +508,12 @@ export const processTool: AgentTool<typeof processSchema> = {
const total = finished.aggregated.length; const total = finished.aggregated.length;
const slice = finished.aggregated.slice( const slice = finished.aggregated.slice(
params.offset ?? 0, params.offset ?? 0,
params.limit params.limit ? (params.offset ?? 0) + params.limit : undefined,
? (params.offset ?? 0) + params.limit
: undefined,
); );
const status = finished.status === "completed" ? "completed" : "failed"; const status =
finished.status === "completed" ? "completed" : "failed";
return { return {
content: [ content: [{ type: "text", text: slice || "(no output recorded)" }],
{ type: "text", text: slice || "(no output recorded)" },
],
details: { details: {
status, status,
sessionId: params.sessionId, sessionId: params.sessionId,
@@ -519,7 +536,10 @@ export const processTool: AgentTool<typeof processSchema> = {
if (!session) { if (!session) {
return { return {
content: [ content: [
{ type: "text", text: `No active session found for ${params.sessionId}` }, {
type: "text",
text: `No active session found for ${params.sessionId}`,
},
], ],
details: { status: "failed" }, details: { status: "failed" },
}; };
@@ -572,7 +592,10 @@ export const processTool: AgentTool<typeof processSchema> = {
if (!session) { if (!session) {
return { return {
content: [ content: [
{ type: "text", text: `No active session found for ${params.sessionId}` }, {
type: "text",
text: `No active session found for ${params.sessionId}`,
},
], ],
details: { status: "failed" }, details: { status: "failed" },
}; };

View File

@@ -11,9 +11,20 @@ export function getShellConfig(): { shell: string; args: string[] } {
} }
export function sanitizeBinaryOutput(text: string): string { export function sanitizeBinaryOutput(text: string): string {
return text const scrubbed = text.replace(/[\p{Format}\p{Surrogate}]/gu, "");
.replace(/[\p{Format}\p{Surrogate}]/gu, "") if (!scrubbed) return scrubbed;
.replace(/[\x00-\x08\x0B\x0C\x0E-\x1F]/g, ""); const chunks: string[] = [];
for (const char of scrubbed) {
const code = char.codePointAt(0);
if (code == null) continue;
if (code === 0x09 || code === 0x0a || code === 0x0d) {
chunks.push(char);
continue;
}
if (code < 0x20) continue;
chunks.push(char);
}
return chunks.join("");
} }
export function killProcessTree(pid: number): void { export function killProcessTree(pid: number): void {