fix: harden skill install timeouts

This commit is contained in:
Peter Steinberger 2026-01-08 03:54:06 +01:00
parent f1a09febbf
commit 69d8e46b84
5 changed files with 112 additions and 15 deletions

View File

@ -16,6 +16,7 @@
- Groups: `whatsapp.groups`, `telegram.groups`, and `imessage.groups` now act as allowlists when set. Add `"*"` to keep allow-all behavior. - Groups: `whatsapp.groups`, `telegram.groups`, and `imessage.groups` now act as allowlists when set. Add `"*"` to keep allow-all behavior.
### Fixes ### Fixes
- Skills: harden skill installs with process-group timeouts, clearer timeout messaging, and logged rerun commands when installs fail.
- Heartbeat: default interval now 30m with a new default prompt + HEARTBEAT.md template. - Heartbeat: default interval now 30m with a new default prompt + HEARTBEAT.md template.
- Onboarding: write auth profiles to the multi-agent path (`~/.clawdbot/agents/main/agent/`) so the gateway finds credentials on first startup. Thanks @minghinmatthewlam for PR #327. - Onboarding: write auth profiles to the multi-agent path (`~/.clawdbot/agents/main/agent/`) so the gateway finds credentials on first startup. Thanks @minghinmatthewlam for PR #327.
- Docs: add missing `ui:install` setup step in the README. Thanks @hugobarauna for PR #300. - Docs: add missing `ui:install` setup step in the README. Thanks @hugobarauna for PR #300.

View File

@ -27,6 +27,7 @@ export type SkillInstallResult = {
stdout: string; stdout: string;
stderr: string; stderr: string;
code: number | null; code: number | null;
command?: string;
}; };
function summarizeInstallOutput(text: string): string | undefined { function summarizeInstallOutput(text: string): string | undefined {
@ -55,7 +56,22 @@ function formatInstallFailureMessage(result: {
code: number | null; code: number | null;
stdout: string; stdout: string;
stderr: string; stderr: string;
signal?: NodeJS.Signals | null;
killed?: boolean;
timedOut?: boolean;
timeoutMs?: number;
}): string { }): string {
const isTimeout =
result.timedOut || result.signal === "SIGKILL" || result.killed === true;
if (isTimeout) {
const seconds =
typeof result.timeoutMs === "number"
? Math.max(1, Math.round(result.timeoutMs / 1000))
: undefined;
return seconds
? `Install timed out after ${seconds}s`
: "Install timed out";
}
const code = const code =
typeof result.code === "number" ? `exit ${result.code}` : "unknown exit"; typeof result.code === "number" ? `exit ${result.code}` : "unknown exit";
const summary = const summary =
@ -127,6 +143,15 @@ function buildInstallCommand(
} }
} }
function formatCommand(argv: string[]): string {
return argv
.map((arg) => {
if (/^[A-Za-z0-9_./:=+-]+$/.test(arg)) return arg;
return `'${arg.replace(/'/g, `'\\''`)}'`;
})
.join(" ");
}
async function resolveBrewBinDir( async function resolveBrewBinDir(
timeoutMs: number, timeoutMs: number,
): Promise<string | undefined> { ): Promise<string | undefined> {
@ -194,6 +219,16 @@ export async function installSkill(
code: null, code: null,
}; };
} }
if (!command.argv || command.argv.length === 0) {
return {
ok: false,
message: "invalid install command",
stdout: "",
stderr: "",
code: null,
};
}
const commandLine = formatCommand(command.argv);
if (spec.kind === "brew" && !hasBinary("brew")) { if (spec.kind === "brew" && !hasBinary("brew")) {
return { return {
ok: false, ok: false,
@ -201,6 +236,7 @@ export async function installSkill(
stdout: "", stdout: "",
stderr: "", stderr: "",
code: null, code: null,
command: commandLine,
}; };
} }
if (spec.kind === "uv" && !hasBinary("uv")) { if (spec.kind === "uv" && !hasBinary("uv")) {
@ -218,6 +254,7 @@ export async function installSkill(
stdout: brewResult.stdout.trim(), stdout: brewResult.stdout.trim(),
stderr: brewResult.stderr.trim(), stderr: brewResult.stderr.trim(),
code: brewResult.code, code: brewResult.code,
command: formatCommand(["brew", "install", "uv"]),
}; };
} }
} else { } else {
@ -227,18 +264,10 @@ export async function installSkill(
stdout: "", stdout: "",
stderr: "", stderr: "",
code: null, code: null,
command: commandLine,
}; };
} }
} }
if (!command.argv || command.argv.length === 0) {
return {
ok: false,
message: "invalid install command",
stdout: "",
stderr: "",
code: null,
};
}
if (spec.kind === "go" && !hasBinary("go")) { if (spec.kind === "go" && !hasBinary("go")) {
if (hasBinary("brew")) { if (hasBinary("brew")) {
@ -255,6 +284,7 @@ export async function installSkill(
stdout: brewResult.stdout.trim(), stdout: brewResult.stdout.trim(),
stderr: brewResult.stderr.trim(), stderr: brewResult.stderr.trim(),
code: brewResult.code, code: brewResult.code,
command: formatCommand(["brew", "install", "go"]),
}; };
} }
} else { } else {
@ -264,6 +294,7 @@ export async function installSkill(
stdout: "", stdout: "",
stderr: "", stderr: "",
code: null, code: null,
command: commandLine,
}; };
} }
} }
@ -277,7 +308,14 @@ export async function installSkill(
const result = await (async () => { const result = await (async () => {
const argv = command.argv; const argv = command.argv;
if (!argv || argv.length === 0) { if (!argv || argv.length === 0) {
return { code: null, stdout: "", stderr: "invalid install command" }; return {
code: null,
stdout: "",
stderr: "invalid install command",
signal: null,
killed: false,
timedOut: false,
};
} }
try { try {
return await runCommandWithTimeout(argv, { return await runCommandWithTimeout(argv, {
@ -286,16 +324,26 @@ export async function installSkill(
}); });
} catch (err) { } catch (err) {
const stderr = err instanceof Error ? err.message : String(err); const stderr = err instanceof Error ? err.message : String(err);
return { code: null, stdout: "", stderr }; return {
code: null,
stdout: "",
stderr,
signal: null,
killed: false,
timedOut: false,
};
} }
})(); })();
const success = result.code === 0; const success = result.code === 0 && !result.timedOut;
return { return {
ok: success, ok: success,
message: success ? "Installed" : formatInstallFailureMessage(result), message: success
? "Installed"
: formatInstallFailureMessage({ ...result, timeoutMs }),
stdout: result.stdout.trim(), stdout: result.stdout.trim(),
stderr: result.stderr.trim(), stderr: result.stderr.trim(),
code: result.code, code: result.code,
command: commandLine,
}; };
} }

View File

@ -163,6 +163,9 @@ export async function setupSkills(
spin.stop( spin.stop(
`Install failed: ${name}${code}${detail ? `${detail}` : ""}`, `Install failed: ${name}${code}${detail ? `${detail}` : ""}`,
); );
if (result.command) {
runtime.log(`Command: ${result.command}`);
}
if (result.stderr) runtime.log(result.stderr.trim()); if (result.stderr) runtime.log(result.stderr.trim());
else if (result.stdout) runtime.log(result.stdout.trim()); else if (result.stdout) runtime.log(result.stdout.trim());
runtime.log( runtime.log(

View File

@ -18,5 +18,16 @@ describe("runCommandWithTimeout", () => {
expect(result.code).toBe(0); expect(result.code).toBe(0);
expect(result.stdout).toBe("ok"); expect(result.stdout).toBe("ok");
expect(result.timedOut).toBe(false);
});
it("marks timed out processes", async () => {
if (process.platform === "win32") return;
const result = await runCommandWithTimeout(
[process.execPath, "-e", "setTimeout(() => {}, 1000)"],
{ timeoutMs: 50 },
);
expect(result.timedOut).toBe(true);
}); });
}); });

View File

@ -41,6 +41,7 @@ export type SpawnResult = {
code: number | null; code: number | null;
signal: NodeJS.Signals | null; signal: NodeJS.Signals | null;
killed: boolean; killed: boolean;
timedOut: boolean;
}; };
export type CommandOptions = { export type CommandOptions = {
@ -60,18 +61,42 @@ export async function runCommandWithTimeout(
: optionsOrTimeout; : optionsOrTimeout;
const { timeoutMs, cwd, input, env } = options; const { timeoutMs, cwd, input, env } = options;
const supportsGroupKill = process.platform !== "win32";
const killGraceMs = 5_000;
// Spawn with inherited stdin (TTY) so tools like `pi` stay interactive when needed. // Spawn with inherited stdin (TTY) so tools like `pi` stay interactive when needed.
return await new Promise((resolve, reject) => { return await new Promise((resolve, reject) => {
const child = spawn(argv[0], argv.slice(1), { const child = spawn(argv[0], argv.slice(1), {
stdio: [input ? "pipe" : "inherit", "pipe", "pipe"], stdio: [input ? "pipe" : "inherit", "pipe", "pipe"],
cwd, cwd,
env: env ? { ...process.env, ...env } : process.env, env: env ? { ...process.env, ...env } : process.env,
detached: supportsGroupKill,
}); });
let stdout = ""; let stdout = "";
let stderr = ""; let stderr = "";
let settled = false; let settled = false;
let timedOut = false;
let killTimer: NodeJS.Timeout | undefined;
const timer = setTimeout(() => { const timer = setTimeout(() => {
child.kill("SIGKILL"); if (settled) return;
timedOut = true;
const pid = child.pid;
if (pid) {
try {
if (supportsGroupKill) process.kill(-pid, "SIGTERM");
else child.kill("SIGTERM");
} catch {
// ignore
}
killTimer = setTimeout(() => {
try {
if (supportsGroupKill) process.kill(-pid, "SIGKILL");
else child.kill("SIGKILL");
} catch {
// ignore
}
}, killGraceMs);
}
}, timeoutMs); }, timeoutMs);
if (input && child.stdin) { if (input && child.stdin) {
@ -89,13 +114,22 @@ export async function runCommandWithTimeout(
if (settled) return; if (settled) return;
settled = true; settled = true;
clearTimeout(timer); clearTimeout(timer);
if (killTimer) clearTimeout(killTimer);
reject(err); reject(err);
}); });
child.on("close", (code, signal) => { child.on("close", (code, signal) => {
if (settled) return; if (settled) return;
settled = true; settled = true;
clearTimeout(timer); clearTimeout(timer);
resolve({ stdout, stderr, code, signal, killed: child.killed }); if (killTimer) clearTimeout(killTimer);
resolve({
stdout,
stderr,
code,
signal,
killed: child.killed,
timedOut,
});
}); });
}); });
} }