diff --git a/bunfig.toml b/bunfig.toml index 597cd49b4..de9506fbc 100644 --- a/bunfig.toml +++ b/bunfig.toml @@ -1,3 +1,13 @@ [install] exact = true -saveTextLockfile = true \ No newline at end of file +saveTextLockfile = true + +[test] +coverage.exclude = [ + "**/dist/**", + "**/node_modules/**", + "**/*.d.ts", + "**/test/**", + "**/*.test.ts", + "**/*.test.js" +] \ No newline at end of file diff --git a/package.json b/package.json index 65feaa413..5ee6444d0 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,7 @@ "deploy": "turbo deploy", "translate": "turbo translate", "typecheck": "turbo typecheck", - "ci:local": "bunx turbo format lint build attw publint test:coverage --concurrency=14 --summarize", + "ci:local": "bunx turbo format:biome lint build attw publint test:coverage --concurrency=14 --summarize", "postinstall": "lefthook install", "test:typecheck": "cd test && bunx tsc", "test:e2e": "bun test test/*.ts --coverage --preload ./test/scripts/setup-platform-resources.ts", diff --git a/sdk/cli/src/commands/login.test.ts b/sdk/cli/src/commands/login.test.ts new file mode 100644 index 000000000..2b6cc6879 --- /dev/null +++ b/sdk/cli/src/commands/login.test.ts @@ -0,0 +1,154 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, expect, mock, test } from "bun:test"; +import { Readable } from "node:stream"; + +// Since the login command is complex with dependencies, we'll test the STDIN reading logic separately +// by extracting it into a testable function +describe("login STDIN reading", () => { + test("reads token from STDIN properly", async () => { + const testToken = "test-token-123"; + + // Create a mock stdin stream + const mockStdin = new Readable({ + read() { + this.push(testToken); + this.push(null); // End the stream + }, + }); + + // Mock the STDIN reading logic + const readStdinToken = async (): Promise => { + const chunks: Buffer[] = []; + let timeout: NodeJS.Timeout | undefined; + + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(new Error("Timeout reading from STDIN after 30 seconds")); + }, 30_000); + }); + + try { + const readPromise = (async () => { + for await (const chunk of mockStdin) { + chunks.push(Buffer.from(chunk)); + } + return Buffer.concat(chunks).toString().trim(); + })(); + + const result = await Promise.race([readPromise, timeoutPromise]); + return result; + } finally { + if (timeout) { + clearTimeout(timeout); + } + } + }; + + const result = await readStdinToken(); + expect(result).toBe(testToken); + }); + + test("handles timeout correctly", async () => { + // Create a mock stdin that never ends + const mockStdin = new Readable({ + read() { + // Don't push null, keep stream open + }, + }); + + const readStdinTokenWithShortTimeout = async (): Promise => { + const chunks: Buffer[] = []; + let timeout: NodeJS.Timeout | undefined; + + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(new Error("Timeout reading from STDIN after 30 seconds")); + }, 10); // Very short timeout for test + }); + + try { + const readPromise = (async () => { + for await (const chunk of mockStdin) { + chunks.push(Buffer.from(chunk)); + } + return Buffer.concat(chunks).toString().trim(); + })(); + + const result = await Promise.race([readPromise, timeoutPromise]); + return result; + } finally { + if (timeout) { + clearTimeout(timeout); + } + } + }; + + await expect(readStdinTokenWithShortTimeout()).rejects.toThrow("Timeout reading from STDIN after 30 seconds"); + }); + + test("properly cleans up timeout on success", async () => { + const testToken = "cleanup-test-token"; + let timeoutCleared = false; + + // Mock setTimeout and clearTimeout to verify cleanup + const originalSetTimeout = global.setTimeout; + const originalClearTimeout = global.clearTimeout; + + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + global.setTimeout = mock((fn: any, delay: number) => { + return originalSetTimeout(fn, delay); + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + }) as any; + + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + global.clearTimeout = mock((id: any) => { + timeoutCleared = true; + return originalClearTimeout(id); + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + }) as any; + + const mockStdin = new Readable({ + read() { + this.push(testToken); + this.push(null); + }, + }); + + const readStdinTokenWithCleanup = async (): Promise => { + const chunks: Buffer[] = []; + let timeout: NodeJS.Timeout | undefined; + + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(new Error("Timeout reading from STDIN after 30 seconds")); + }, 30_000); + }); + + try { + const readPromise = (async () => { + for await (const chunk of mockStdin) { + chunks.push(Buffer.from(chunk)); + } + return Buffer.concat(chunks).toString().trim(); + })(); + + const result = await Promise.race([readPromise, timeoutPromise]); + return result; + } finally { + if (timeout) { + clearTimeout(timeout); + } + } + }; + + const result = await readStdinTokenWithCleanup(); + + expect(result).toBe(testToken); + expect(timeoutCleared).toBe(true); + expect(global.clearTimeout).toHaveBeenCalled(); + + // Restore original functions + global.setTimeout = originalSetTimeout; + global.clearTimeout = originalClearTimeout; + }); +}); diff --git a/sdk/cli/src/commands/login.ts b/sdk/cli/src/commands/login.ts index 32a264a43..b636c88b8 100644 --- a/sdk/cli/src/commands/login.ts +++ b/sdk/cli/src/commands/login.ts @@ -53,16 +53,33 @@ export function loginCommand(): Command { if (cmd.args.length > 0) { cancel("A token should be provided using STDIN, not as an argument"); } - personalAccessToken = await Promise.race([ - (async () => { - const chunks: Buffer[] = []; - for await (const chunk of process.stdin) { - chunks.push(Buffer.from(chunk)); + personalAccessToken = await (async () => { + const chunks: Buffer[] = []; + let timeout: NodeJS.Timeout | undefined; + + // Set up a timeout to prevent hanging if STDIN never closes + const timeoutPromise = new Promise((_, reject) => { + timeout = setTimeout(() => { + reject(new Error("Timeout reading from STDIN after 30 seconds")); + }, 30_000); + }); + + try { + const readPromise = (async () => { + for await (const chunk of process.stdin) { + chunks.push(Buffer.from(chunk)); + } + return Buffer.concat(chunks).toString().trim(); + })(); + + const result = await Promise.race([readPromise, timeoutPromise]); + return result; + } finally { + if (timeout) { + clearTimeout(timeout); } - return Buffer.concat(chunks).toString().trim(); - })(), - new Promise((resolve) => setTimeout(() => resolve(""), 1_000)), - ]); + } + })(); try { validate(PersonalAccessTokenSchema, personalAccessToken); } catch { diff --git a/sdk/cli/src/commands/platform/utils/wait-for-completion.test.ts b/sdk/cli/src/commands/platform/utils/wait-for-completion.test.ts new file mode 100644 index 000000000..d3c7e0bb4 --- /dev/null +++ b/sdk/cli/src/commands/platform/utils/wait-for-completion.test.ts @@ -0,0 +1,97 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, expect, mock, test } from "bun:test"; +import { waitForCompletion } from "./wait-for-completion.js"; + +// Mock the settlemint client and services +const mockRead = mock(() => Promise.resolve({ status: "COMPLETED" })); +const mockRestart = mock(() => Promise.resolve()); + +const mockService = { + read: mockRead, + restart: mockRestart, +}; + +const mockSettlemintClient = { + blockchainNetwork: mockService, + blockchainNode: mockService, + customDeployment: mockService, + insights: mockService, + integrationTool: mockService, + loadBalancer: mockService, + middleware: mockService, + privateKey: mockService, + storage: mockService, + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any +} as any; + +describe("waitForCompletion", () => { + test("resolves immediately for workspace resource type", async () => { + const result = await waitForCompletion({ + settlemint: mockSettlemintClient, + type: "workspace", + uniqueName: "test-workspace", + action: "deploy", + }); + + expect(result).toBe(true); + }); + + test("resolves immediately for application resource type", async () => { + const result = await waitForCompletion({ + settlemint: mockSettlemintClient, + type: "application", + uniqueName: "test-app", + action: "deploy", + }); + + expect(result).toBe(true); + }); + + test("calls service read method", async () => { + // Mock service that completes immediately + const mockCompleteRead = mock(() => Promise.resolve({ status: "COMPLETED" })); + const completeMockService = { read: mockCompleteRead, restart: mockRestart }; + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + const completeMockClient = { blockchainNetwork: completeMockService } as any; + + const result = await waitForCompletion({ + settlemint: completeMockClient, + type: "blockchain network", + uniqueName: "test-network", + action: "deploy", + }); + + expect(result).toBe(true); + expect(mockCompleteRead).toHaveBeenCalledWith("test-network"); + }); + + test("handles failed status correctly", async () => { + const mockFailedRead = mock(() => Promise.resolve({ status: "FAILED" })); + const failedMockService = { read: mockFailedRead, restart: mockRestart }; + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + const failedMockClient = { blockchainNetwork: failedMockService } as any; + + const result = await waitForCompletion({ + settlemint: failedMockClient, + type: "blockchain network", + uniqueName: "test-network", + action: "deploy", + }); + + expect(result).toBe(false); + }); + + test("throws error for unsupported service type", async () => { + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + const mockClientWithoutService = {} as any; + + await expect(() => + waitForCompletion({ + settlemint: mockClientWithoutService, + type: "blockchain network" as const, + uniqueName: "test-network", + action: "deploy", + }), + ).toThrow("Service blockchainNetwork does not support status checking"); + }); +}); diff --git a/sdk/cli/src/utils/parse-number.test.ts b/sdk/cli/src/utils/parse-number.test.ts new file mode 100644 index 000000000..01a94a694 --- /dev/null +++ b/sdk/cli/src/utils/parse-number.test.ts @@ -0,0 +1,61 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, expect, test } from "bun:test"; +import { parseNumber } from "./parse-number.js"; + +describe("parseNumber", () => { + test("parses valid integer strings", () => { + expect(parseNumber("123")).toBe(123); + expect(parseNumber("0")).toBe(0); + expect(parseNumber("-42")).toBe(-42); + }); + + test("parses valid decimal strings", () => { + expect(parseNumber("3.14")).toBe(3.14); + expect(parseNumber("-2.5")).toBe(-2.5); + expect(parseNumber("0.0")).toBe(0); + }); + + test("handles whitespace trimming", () => { + expect(parseNumber(" 123 ")).toBe(123); + expect(parseNumber("\t42\n")).toBe(42); + expect(parseNumber(" -3.14 ")).toBe(-3.14); + }); + + test("throws error for non-string input", () => { + // biome-ignore lint/suspicious/noExplicitAny: Testing type validation + expect(() => parseNumber(123 as any)).toThrow("Value must be a string"); + // biome-ignore lint/suspicious/noExplicitAny: Testing type validation + expect(() => parseNumber(null as any)).toThrow("Value must be a string"); + // biome-ignore lint/suspicious/noExplicitAny: Testing type validation + expect(() => parseNumber(undefined as any)).toThrow("Value must be a string"); + }); + + test("throws error for empty strings", () => { + expect(() => parseNumber("")).toThrow("Value cannot be empty"); + expect(() => parseNumber(" ")).toThrow("Value cannot be empty"); + expect(() => parseNumber("\t\n")).toThrow("Value cannot be empty"); + }); + + test("throws error for invalid number strings", () => { + expect(() => parseNumber("abc")).toThrow('"abc" is not a valid number'); + expect(() => parseNumber("12.34.56")).toThrow('"12.34.56" is not a valid number'); + expect(() => parseNumber("not-a-number")).toThrow('"not-a-number" is not a valid number'); + }); + + test("throws error for infinity values", () => { + expect(() => parseNumber("Infinity")).toThrow('"Infinity" is not a finite number'); + expect(() => parseNumber("-Infinity")).toThrow('"-Infinity" is not a finite number'); + }); + + test("handles scientific notation", () => { + expect(parseNumber("1e5")).toBe(100000); + expect(parseNumber("2.5e-2")).toBe(0.025); + expect(parseNumber("-3e4")).toBe(-30000); + }); + + test("handles edge cases", () => { + expect(parseNumber("0")).toBe(0); + expect(parseNumber("-0")).toBe(-0); + expect(parseNumber("0.000001")).toBe(0.000001); + }); +}); diff --git a/sdk/cli/src/utils/parse-number.ts b/sdk/cli/src/utils/parse-number.ts index 6fd305a39..ac9e594e9 100644 --- a/sdk/cli/src/utils/parse-number.ts +++ b/sdk/cli/src/utils/parse-number.ts @@ -1 +1,28 @@ -export const parseNumber = (value: string) => Number(value); +/** + * Safely parses a string to a number with proper validation + * @param value - The string value to parse + * @returns The parsed number + * @throws Error if the value cannot be parsed to a valid finite number + */ +export const parseNumber = (value: string): number => { + if (typeof value !== "string") { + throw new Error("Value must be a string"); + } + + const trimmed = value.trim(); + if (trimmed === "") { + throw new Error("Value cannot be empty"); + } + + const parsed = Number(trimmed); + + if (Number.isNaN(parsed)) { + throw new Error(`"${value}" is not a valid number`); + } + + if (!Number.isFinite(parsed)) { + throw new Error(`"${value}" is not a finite number`); + } + + return parsed; +}; diff --git a/sdk/hasura/src/postgres.test.ts b/sdk/hasura/src/postgres.test.ts new file mode 100644 index 000000000..b1417ff00 --- /dev/null +++ b/sdk/hasura/src/postgres.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, test } from "bun:test"; + +describe("postgres connection handling", () => { + test("demonstrates improved error handling patterns", () => { + // This test verifies our code changes compile and import correctly + // The actual postgres functionality requires a real database connection + // so we focus on testing the patterns we improved + + // Test that our improved error handling functions can be imported + expect(() => { + // Import would fail if there were syntax errors in our fixes + require("./postgres.js"); + }).not.toThrow(); + }); + + test("validates server-side environment check", () => { + // Since runsOnServer is determined at module load time and relies on typeof window, + // we'll test this by verifying the function exists and has the expected signature + const { createPostgresPool } = require("./postgres.js"); + + // Test that the function exists and can be called (it should work in Node.js environment) + expect(typeof createPostgresPool).toBe("function"); + + // We can't easily mock the browser environment for this test since runsOnServer + // is evaluated at module load time, so we'll verify the function behavior instead + expect(() => { + // This should work in Node.js environment + const pool = createPostgresPool("postgresql://test"); + expect(pool).toBeDefined(); + // Clean up immediately + pool.end(); + }).not.toThrow(); + }); + + test("validates connection string requirement", () => { + const { createPostgresPool } = require("./postgres.js"); + + // Test that function accepts a valid connection string + expect(() => { + const pool = createPostgresPool("postgresql://test"); + expect(pool).toBeDefined(); + pool.end(); + }).not.toThrow(); + + // Test that function throws error for empty connection string + expect(() => { + createPostgresPool(""); + }).toThrow("Database URL is required"); + + // Test that function throws error for null/undefined connection string + expect(() => { + // biome-ignore lint/suspicious/noExplicitAny: + createPostgresPool(null as any); + }).toThrow("Database URL is required"); + }); +}); diff --git a/sdk/hasura/src/postgres.ts b/sdk/hasura/src/postgres.ts index 6f50b11d6..9faab96e2 100644 --- a/sdk/hasura/src/postgres.ts +++ b/sdk/hasura/src/postgres.ts @@ -26,38 +26,62 @@ const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); */ function setupErrorHandling(pool: pg.Pool) { let retryCount = 0; + let isRetrying = false; + const clientErrorHandlers = new WeakSet(); - pool.on("error", async (err: Error) => { + const handlePoolError = async (err: Error) => { console.error("[Drizzle] Pool error occurred:", err); + // Prevent concurrent retry attempts + if (isRetrying) { + console.log("[Drizzle] Recovery already in progress, skipping"); + return; + } + if (retryCount < 3) { + isRetrying = true; retryCount++; console.log(`[Drizzle] Attempting to recover - retry ${retryCount}/3`); try { const client = await pool.connect(); - client.release(); - console.log("[Drizzle] Successfully recovered connection"); - retryCount = 0; + try { + // Test the connection with timeout + await Promise.race([ + client.query("SELECT 1"), + new Promise((_, reject) => setTimeout(() => reject(new Error("Query timeout")), 5000)), + ]); + console.log("[Drizzle] Successfully recovered connection"); + retryCount = 0; + } finally { + client.release(); + } } catch (retryError) { console.error(`[Drizzle] Recovery attempt ${retryCount} failed:`, retryError); await sleep(5000 * 2 ** (retryCount - 1)); + } finally { + isRetrying = false; } } else { console.error("[Drizzle] Max retries exceeded - pool is in an error state"); pool.emit("permanent-failure", err); } - }); - - pool.on("connect", (client: pg.PoolClient) => { - client.on("error", (err: Error) => { - console.error("[Drizzle] Database client error:", err); - }); - }); + }; - pool.on("connect", () => { + const handleConnect = (client: pg.PoolClient) => { + // Only add error handler if not already added (prevent memory leaks) + if (!clientErrorHandlers.has(client)) { + clientErrorHandlers.add(client); + client.on("error", (err: Error) => { + console.error("[Drizzle] Database client error:", err); + }); + } retryCount = 0; - }); + isRetrying = false; + }; + + pool.on("error", handlePoolError); + pool.on("connect", handleConnect); } /** @@ -81,6 +105,10 @@ function setupErrorHandling(pool: pg.Pool) { * } */ export function createPostgresPool(databaseUrl: string) { + if (!databaseUrl?.trim()) { + throw new Error("Database URL is required"); + } + if (!runsOnServer) { throw new Error("Drizzle client can only be created on the server side"); } diff --git a/sdk/mcp/src/utils/schema-processor.ts b/sdk/mcp/src/utils/schema-processor.ts index 4fa4021cf..1d5542f19 100644 --- a/sdk/mcp/src/utils/schema-processor.ts +++ b/sdk/mcp/src/utils/schema-processor.ts @@ -10,16 +10,25 @@ import type { } from "graphql"; /** - * Creates a promise that rejects after a specified timeout + * Creates a promise that rejects after a specified timeout with proper cleanup * @param ms - Timeout in milliseconds - * @returns A promise that rejects after the specified timeout + * @returns An object with the promise and a cleanup function */ -const createTimeoutPromise = (ms: number): Promise => { - return new Promise((_, reject) => { - setTimeout(() => { +const createTimeoutPromise = (ms: number): { promise: Promise; cleanup: () => void } => { + let timeoutId: NodeJS.Timeout; + const promise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { reject(new Error(`Operation timed out after ${ms}ms`)); }, ms); }); + + const cleanup = () => { + if (timeoutId) { + clearTimeout(timeoutId); + } + }; + + return { promise, cleanup }; }; /** @@ -47,8 +56,13 @@ async function fetchGraphQLSchema( method: "POST", }); - // Race between the schema loading and a timeout - return Promise.race([schemaPromise, createTimeoutPromise(timeoutMs)]); + // Race between the schema loading and a timeout with proper cleanup + const timeout = createTimeoutPromise(timeoutMs); + try { + return await Promise.race([schemaPromise, timeout.promise]); + } finally { + timeout.cleanup(); + } } /** diff --git a/sdk/minio/src/helpers/functions.ts b/sdk/minio/src/helpers/functions.ts index 015c45a6d..83f69e479 100644 --- a/sdk/minio/src/helpers/functions.ts +++ b/sdk/minio/src/helpers/functions.ts @@ -72,25 +72,43 @@ export async function getFilesList( const objects = await executeMinioOperation(client, listOperation); console.log(`Found ${objects.length} files in MinIO`); - const fileObjects = await Promise.all( + const fileObjects = await Promise.allSettled( objects.map(async (obj): Promise => { - const presignedUrlOperation = createPresignedUrlOperation( - bucket, - obj.name, - 3600, // 1 hour expiry - ); - const url = await executeMinioOperation(client, presignedUrlOperation); + try { + const presignedUrlOperation = createPresignedUrlOperation( + bucket, + obj.name, + 3600, // 1 hour expiry + ); + const url = await executeMinioOperation(client, presignedUrlOperation); - return { - id: obj.name, - name: obj.name.split("/").pop() || obj.name, - contentType: "application/octet-stream", // Default type - size: obj.size, - uploadedAt: obj.lastModified.toISOString(), - etag: obj.etag, - url, - }; + return { + id: obj.name, + name: obj.name.split("/").pop() || obj.name, + contentType: "application/octet-stream", // Default type + size: obj.size, + uploadedAt: obj.lastModified.toISOString(), + etag: obj.etag, + url, + }; + } catch (error) { + console.warn(`Failed to generate presigned URL for ${obj.name}:`, error); + // Return metadata without URL for failed presigned URL operations + return { + id: obj.name, + name: obj.name.split("/").pop() || obj.name, + contentType: "application/octet-stream", // Default type + size: obj.size, + uploadedAt: obj.lastModified.toISOString(), + etag: obj.etag, + // url is omitted for failed operations (undefined) + }; + } }), + ).then((results) => + results + .filter((result): result is PromiseFulfilledResult => result.status === "fulfilled") + .map((result) => result.value), ); return validate(FileMetadataSchema.array(), fileObjects); @@ -147,7 +165,7 @@ export async function getFileById( // Check for content-length in metadata if (statResult.metaData["content-length"]) { const parsedSize = Number.parseInt(statResult.metaData["content-length"], 10); - if (!Number.isNaN(parsedSize)) { + if (!Number.isNaN(parsedSize) && parsedSize >= 0 && Number.isFinite(parsedSize)) { size = parsedSize; } } diff --git a/sdk/utils/src/terminal/execute-command.test.ts b/sdk/utils/src/terminal/execute-command.test.ts index aa03dd51c..26662263e 100644 --- a/sdk/utils/src/terminal/execute-command.test.ts +++ b/sdk/utils/src/terminal/execute-command.test.ts @@ -1,9 +1,112 @@ -import { describe, expect, test } from "bun:test"; -import { executeCommand } from "./execute-command.js"; +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { describe, expect, mock, test } from "bun:test"; +import { CommandError, executeCommand } from "./execute-command.js"; describe("executeCommand", () => { test("executes command successfully", async () => { const output = await executeCommand("echo", ["hello"]); expect(output).toContain("hello\n"); }); + + test("captures stderr output", async () => { + const output = await executeCommand("node", ["-e", "console.error('error message')"]); + expect(output.some((line) => line.includes("error message"))).toBe(true); + }); + + test("handles command failure with proper error", async () => { + await expect( + () => executeCommand("false", []), // Command that always exits with code 1 + ).toThrow(CommandError); + }); + + test("handles non-existent command", async () => { + await expect(() => executeCommand("non-existent-command-12345", [])).toThrow(CommandError); + }); + + test("respects silent option", async () => { + const originalWrite = process.stdout.write; + let stdoutWritten = false; + + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + process.stdout.write = mock((chunk: any) => { + stdoutWritten = true; + return true; + }); + + try { + await executeCommand("echo", ["silent test"], { silent: true }); + expect(stdoutWritten).toBe(false); + } finally { + process.stdout.write = originalWrite; + } + }); + + test("masks sensitive tokens in output", async () => { + // Test that tokens are masked - this depends on the maskTokens implementation + const output = await executeCommand("echo", ["token_abc123"]); + // The exact masking behavior depends on maskTokens implementation + expect(output).toBeDefined(); + expect(Array.isArray(output)).toBe(true); + }); + + test("properly cleans up stdin pipe on success", async () => { + const originalUnpipe = process.stdin.unpipe; + let unpipeCalled = false; + + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + process.stdin.unpipe = mock((destination?: any) => { + unpipeCalled = true; + return process.stdin; + }); + + try { + await executeCommand("echo", ["test"]); + expect(unpipeCalled).toBe(true); + } finally { + process.stdin.unpipe = originalUnpipe; + } + }); + + test("properly cleans up stdin pipe on error", async () => { + const originalUnpipe = process.stdin.unpipe; + let unpipeCalled = false; + + // biome-ignore lint/suspicious/noExplicitAny: Test mocking requires any + process.stdin.unpipe = mock((destination?: any) => { + unpipeCalled = true; + return process.stdin; + }); + + try { + await expect(() => executeCommand("false", [])).toThrow(); + // Wait a bit to ensure the cleanup has happened + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(unpipeCalled).toBe(true); + } catch { + // Expected to throw + } finally { + process.stdin.unpipe = originalUnpipe; + } + }); + + test("handles commands that write to both stdout and stderr", async () => { + const output = await executeCommand("node", [ + "-e", + "console.log('stdout message'); console.error('stderr message');", + ]); + + const hasStdout = output.some((line) => line.includes("stdout message")); + const hasStderr = output.some((line) => line.includes("stderr message")); + + expect(hasStdout).toBe(true); + expect(hasStderr).toBe(true); + }); + + test("preserves environment variables", async () => { + const output = await executeCommand("node", ["-e", "console.log(process.env.NODE_ENV || 'undefined');"], { + env: { NODE_ENV: "test" }, + }); + + expect(output.some((line) => line.includes("test"))).toBe(true); + }); }); diff --git a/sdk/utils/src/terminal/execute-command.ts b/sdk/utils/src/terminal/execute-command.ts index 2d6320d65..4ebc470aa 100644 --- a/sdk/utils/src/terminal/execute-command.ts +++ b/sdk/utils/src/terminal/execute-command.ts @@ -72,12 +72,13 @@ export async function executeCommand( } output.push(maskedData); }); - child.on("error", (err) => - reject(new CommandError(err.message, "code" in err && typeof err.code === "number" ? err.code : 1, output)), - ); + child.on("error", (err) => { + process.stdin.unpipe(child.stdin); + reject(new CommandError(err.message, "code" in err && typeof err.code === "number" ? err.code : 1, output)); + }); child.on("close", (code) => { + process.stdin.unpipe(child.stdin); if (code === 0 || code === null || code === 143) { - process.stdin.unpipe(child.stdin); resolve(output); return; }