fix(engines): stop a signal-killed child reporting success (#37)

Author: clawedassistant26

src/engines.mjs

@@ -194,6 +194,28 @@ export function runCmd(cmd, args = []) {
   });
 }
 
+/**
+ * True only when a child actually ran and exited 0. Node reports a signal death
+ * as `code === null` (the signal name lands in `signal` instead), so a killed
+ * child — OOM, a timeout wrapper's SIGTERM, Ctrl-C — must not read as success
+ * just because it has no exit code.
+ */
+export function ranOk(r) {
+  return Boolean(r?.ok) && r.code === 0;
+}
+
+/**
+ * Short human reason a `runCmd` result failed: "code 128", "SIGKILL", or the
+ * spawn error message. Null when it succeeded.
+ */
+export function exitReason(r) {
+  if (ranOk(r)) return null;
+  if (r?.error) return r.error.message || String(r.error);
+  if (r?.code != null) return `code ${r.code}`;
+  if (r?.signal) return r.signal;
+  return "unknown error";
+}
+
 /**
  * Hand the current process streams to an external CLI. Arguments, cwd, and the
  * environment are inherited unchanged unless that target explicitly asks for

src/integrations.mjs

@@ -114,7 +114,7 @@ export function printSkillTargets() {
 function summarize(results) {
   for (const r of results) {
     if (r.status === "added" || r.status === "installed") console.log(line(r.key, ok(r.status)));
-    else if (r.status === "failed") console.log(line(r.key, err(`failed${r.code != null ? ` (code ${r.code})` : ""}`)));
+    else if (r.status === "failed") console.log(line(r.key, err(`failed${r.code != null ? ` (code ${r.code})` : r.signal ? ` (${r.signal})` : ""}`)));
     else if (r.status === "not-installed") console.log(line(r.key, ash("not installed — /install " + r.key)));
     else console.log(line(r.key, ash(`skipped — ${r.reason}`)));
   }

src/mcp.mjs

@@ -1,7 +1,7 @@
 // Register MCP (Model Context Protocol) servers across every engine that
 // supports them, from one canonical definition. MoshCode drives each engine's
 // own `mcp add` so the engine owns its config format. See prd/0003.
-import { ENGINES, isInstalled, runCmd } from "./engines.mjs";
+import { ENGINES, isInstalled, ranOk, runCmd } from "./engines.mjs";
 
 // Coding engines that can register MCP servers. Aider has no MCP support.
 export const MCP_ENGINES = ["claude", "gemini", "codex", "opencode"];
@@ -112,7 +112,7 @@ export async function runMcpAdd(plan, { run = runCmd } = {}) {
     if (item.skip) { results.push({ key: item.key, status: "skipped", reason: item.skip }); continue; }
     if (!item.installed) { results.push({ key: item.key, status: "not-installed" }); continue; }
     const r = await run(item.bin, item.argv);
-    results.push({ key: item.key, status: r.ok && (r.code === 0 || r.code == null) ? "added" : "failed", code: r.code });
+    results.push({ key: item.key, status: ranOk(r) ? "added" : "failed", code: r.code, signal: r.signal ?? null });
   }
   return results;
 }

src/skills.mjs

@@ -3,7 +3,7 @@
 // source into its personal skills dir. See prd/0003.
 import os from "node:os";
 import path from "node:path";
-import { ENGINES, isInstalled, runCmd } from "./engines.mjs";
+import { ENGINES, isInstalled, ranOk, runCmd } from "./engines.mjs";
 
 // Coding engines with a skills primitive. Codex/OpenCode/Aider have none.
 export const SKILL_ENGINES = ["claude", "gemini"];
@@ -66,7 +66,7 @@ export async function runSkillInstall(plan, { run = runCmd } = {}) {
     if (item.skip) { results.push({ key: item.key, status: "skipped", reason: item.skip }); continue; }
     if (!item.installed) { results.push({ key: item.key, status: "not-installed" }); continue; }
     const r = await run(item.cmd, item.args);
-    results.push({ key: item.key, status: r.ok && (r.code === 0 || r.code == null) ? "installed" : "failed", code: r.code });
+    results.push({ key: item.key, status: ranOk(r) ? "installed" : "failed", code: r.code, signal: r.signal ?? null });
   }
   return results;
 }

src/upgrade.mjs

@@ -4,7 +4,7 @@
 import { fileURLToPath } from "node:url";
 import path from "node:path";
 import fs from "node:fs";
-import { ENGINES, engineStatus, resolveEngine, upgradeSpec, runCmd } from "./engines.mjs";
+import { ENGINES, engineStatus, exitReason, ranOk, resolveEngine, upgradeSpec, runCmd } from "./engines.mjs";
 import { TOOLS, resolveTool, toolStatus, toolUpgradeSpec } from "./tools.mjs";
 
 // Self-upgrade re-runs the moshcode installer's `update` path. Defaults to the
@@ -125,9 +125,9 @@ export async function runUpgrade(targets = [], io = {}) {
     rule();
     const r = await runCmd(spec.cmd, spec.args);
     rule();
-    const ok = r.ok && (r.code == null || r.code === 0);
-    log(ok ? `✓ ${name} up to date` : `✗ ${name} upgrade failed${r.code != null ? ` (code ${r.code})` : r.error ? `: ${r.error.message || r.error}` : ""}`);
-    results.push({ name, ok, code: r.code });
+    const ok = ranOk(r);
+    log(ok ? `✓ ${name} up to date` : `✗ ${name} upgrade failed (${exitReason(r)})`);
+    results.push({ name, ok, code: r.code, signal: r.signal ?? null });
     return ok;
   };
 

test/engines.test.mjs

@@ -14,7 +14,7 @@ import { fileURLToPath } from "node:url";
 import { spawn } from "node:child_process";
 import test from "node:test";
 
-import { ENGINES, agentLaunchArgs } from "../src/engines.mjs";
+import { ENGINES, agentLaunchArgs, exitReason, ranOk, runCmd } from "../src/engines.mjs";
 
 const BIN = fileURLToPath(new URL("../bin/moshcode.mjs", import.meta.url));
 // The autonomous-session bypass flags each engine declares (engine.agentArgs).
@@ -130,3 +130,30 @@ test("bare engine launch remains a raw passthrough", async () => {
   assert.deepEqual(JSON.parse(result.stdout), ["--model", "sonnet"]);
   assert.equal(result.stderr, "");
 });
+
+test("a signal-killed child is a failure, not a codeless success", async () => {
+  const r = await runCmd("bash", ["-c", "kill -9 $$"]);
+
+  // Node reports a signal death with code === null — the old `code == null`
+  // success check read that as "exited cleanly".
+  assert.equal(r.ok, true);
+  assert.equal(r.code, null);
+  assert.equal(r.signal, "SIGKILL");
+
+  assert.equal(ranOk(r), false);
+  assert.equal(exitReason(r), "SIGKILL");
+});
+
+test("ranOk and exitReason cover clean exits, bad codes, and spawn errors", async () => {
+  const clean = await runCmd("bash", ["-c", "exit 0"]);
+  assert.equal(ranOk(clean), true);
+  assert.equal(exitReason(clean), null);
+
+  const bad = await runCmd("bash", ["-c", "exit 128"]);
+  assert.equal(ranOk(bad), false);
+  assert.equal(exitReason(bad), "code 128");
+
+  const missing = await runCmd("moshcode-does-not-exist-xyz", []);
+  assert.equal(ranOk(missing), false);
+  assert.match(exitReason(missing), /ENOENT|not found|spawn/i);
+});

test/mcp.test.mjs

@@ -142,3 +142,13 @@ fs.writeFileSync(path.join(process.env.CAPS, "${name}.json"), JSON.stringify(pro
   assert.deepEqual(JSON.parse(readFileSync(path.join(caps, "codex.json"), "utf8")), ["mcp", "add", "sentry", "--url", "https://mcp.sentry.dev/mcp"]);
   assert.deepEqual(JSON.parse(readFileSync(path.join(caps, "opencode.json"), "utf8")), ["mcp", "add", "sentry", "--url", "https://mcp.sentry.dev/mcp"]);
 });
+
+test("an mcp add killed by a signal reports failed, not added", async () => {
+  const spec = { name: "s", target: "https://x.dev/mcp", env: [], headers: [] };
+  const plan = planMcpAdd(spec, { installedSet: new Set(["claude"]) });
+  const results = await runMcpAdd(plan, { run: async () => ({ ok: true, code: null, signal: "SIGTERM" }) });
+  const claude = results.find((r) => r.key === "claude");
+
+  assert.equal(claude.status, "failed");
+  assert.equal(claude.signal, "SIGTERM");
+});

test/skills.test.mjs

@@ -64,3 +64,12 @@ test("runSkillInstall summarizes installed / not-installed", async () => {
   assert.equal(byKey.gemini, "installed");
   assert.equal(byKey.claude, "not-installed");
 });
+
+test("a skill install killed by a signal reports failed, not installed", async () => {
+  const plan = planSkillInstall({ source: "https://x/y", name: "y" }, { installedSet: new Set(["gemini"]) });
+  const results = await runSkillInstall(plan, { run: async () => ({ ok: true, code: null, signal: "SIGKILL" }) });
+  const gemini = results.find((r) => r.key === "gemini");
+
+  assert.equal(gemini.status, "failed");
+  assert.equal(gemini.signal, "SIGKILL");
+});