fix snippet execution result handling (#1099)

src/backend/database/routes/snippets-execution.ts

@@ -0,0 +1,29 @@
+export interface SnippetExecutionResult {
+  success: boolean;
+  output: string;
+  error?: string;
+}
+
+export function getSnippetExecutionTimeoutMs(
+  value = process.env.SNIPPET_EXECUTION_TIMEOUT_SECONDS,
+): number | undefined {
+  if (value === undefined || value.trim() === "") return undefined;
+
+  const seconds = Number(value);
+  if (!Number.isFinite(seconds) || seconds <= 0) return undefined;
+
+  return seconds * 1000;
+}
+
+export function createSnippetExecutionResult(
+  exitCode: number | null,
+  output: string,
+  errorOutput: string,
+): SnippetExecutionResult {
+  const success = exitCode === 0 || (exitCode === null && !errorOutput);
+  return {
+    success,
+    output,
+    ...(errorOutput ? { error: errorOutput } : {}),
+  };
+}

src/backend/database/routes/snippets.ts

@@ -5,6 +5,10 @@ import { authLogger, databaseLogger } from "../../utils/logger.js";
 import { AuthManager } from "../../utils/auth-manager.js";
 import { SSH_ALGORITHMS } from "../../utils/ssh-algorithms.js";
 import { extractSnippetReorderUpdates } from "./snippets-reorder.js";
+import {
+  createSnippetExecutionResult,
+  getSnippetExecutionTimeoutMs,
+} from "./snippets-execution.js";
 import { logAudit, getRequestMeta } from "../../utils/audit-logger.js";
 import {
   createCurrentHostResolutionRepository,
@@ -609,10 +613,8 @@ router.post(
         output: string;
         error?: string;
       }>((resolve, reject) => {
-        const timeout = setTimeout(() => {
-          conn.end();
-          reject(new Error("Command execution timeout (30s)"));
-        }, 30000);
+        const timeoutMs = getSnippetExecutionTimeoutMs();
+        let timeout: NodeJS.Timeout | undefined;
 
         conn.on("ready", () => {
           conn.exec(snippet.content, (err, stream) => {
@@ -622,14 +624,21 @@ router.post(
               return reject(err);
             }
 
-            stream.on("close", () => {
+            if (timeoutMs) {
+              timeout = setTimeout(() => {
+                conn.end();
+                reject(
+                  new Error(`Command execution timeout (${timeoutMs / 1000}s)`),
+                );
+              }, timeoutMs);
+            }
+
+            stream.on("close", (exitCode: number | null) => {
               clearTimeout(timeout);
               conn.end();
-              if (errorOutput) {
-                resolve({ success: false, output, error: errorOutput });
-              } else {
-                resolve({ success: true, output });
-              }
+              resolve(
+                createSnippetExecutionResult(exitCode, output, errorOutput),
+              );
             });
 
             stream.on("data", (data: Buffer) => {

src/backend/tests/database/routes/snippets-execution.test.ts

@@ -0,0 +1,48 @@
+import { describe, expect, it } from "vitest";
+import {
+  createSnippetExecutionResult,
+  getSnippetExecutionTimeoutMs,
+} from "../../../database/routes/snippets-execution.js";
+
+describe("snippet execution", () => {
+  it("treats stderr as diagnostic output when the command succeeds", () => {
+    expect(createSnippetExecutionResult(0, "done\n", "warning\n")).toEqual({
+      success: true,
+      output: "done\n",
+      error: "warning\n",
+    });
+  });
+
+  it("uses the exit code to report command failure", () => {
+    expect(createSnippetExecutionResult(1, "", "failed\n")).toEqual({
+      success: false,
+      output: "",
+      error: "failed\n",
+    });
+  });
+
+  it("preserves the previous fallback when no exit code is available", () => {
+    expect(createSnippetExecutionResult(null, "done\n", "")).toEqual({
+      success: true,
+      output: "done\n",
+    });
+    expect(createSnippetExecutionResult(null, "", "failed\n").success).toBe(
+      false,
+    );
+  });
+
+  it("disables the command timeout by default", () => {
+    expect(getSnippetExecutionTimeoutMs(undefined)).toBeUndefined();
+  });
+
+  it("converts a configured timeout from seconds to milliseconds", () => {
+    expect(getSnippetExecutionTimeoutMs("45")).toBe(45_000);
+  });
+
+  it.each(["", "0", "-1", "invalid"])(
+    "ignores invalid timeout value %j",
+    (value) => {
+      expect(getSnippetExecutionTimeoutMs(value)).toBeUndefined();
+    },
+  );
+});