diff --git a/README.md b/README.md index f43cc2b..86fb6a4 100644 --- a/README.md +++ b/README.md @@ -80,6 +80,7 @@ Peekaboo can be configured using environment variables: ```json { + "PEEKABOO_AI_PROVIDERS": "ollama/llava:latest,openai/gpt-4o", "PEEKABOO_LOG_LEVEL": "debug", "PEEKABOO_LOG_FILE": "~/Library/Logs/peekaboo-mcp-debug.log", "PEEKABOO_DEFAULT_SAVE_PATH": "~/Pictures/PeekabooCaptures", diff --git a/src/utils/peekaboo-cli.ts b/src/utils/peekaboo-cli.ts index a9393da..27cc1eb 100644 --- a/src/utils/peekaboo-cli.ts +++ b/src/utils/peekaboo-cli.ts @@ -142,14 +142,10 @@ export async function executeSwiftCli( // Kill the process with SIGTERM first try { - try { process.kill('SIGTERM'); } catch (err) { // Process might already be dead } - } catch (err) { - // Process might already be dead - } // Give it a moment to terminate gracefully, then force kill setTimeout(() => { @@ -165,7 +161,11 @@ export async function executeSwiftCli( resolve({ success: false, - error: `Command timed out after ${timeoutMs}ms: ${cliPath} ${args.join(' ')}` + error: { + message: `Swift CLI execution timed out after ${timeoutMs}ms. This may indicate a permission dialog is waiting for user input, or the process is stuck.`, + code: "SWIFT_CLI_TIMEOUT", + details: `Command: ${cliPath} ${fullArgs.join(' ')}` + } }); } }, timeoutMs); diff --git a/tests/integration/image-tool.test.ts b/tests/integration/image-tool.test.ts index 6032c0d..25df92e 100644 --- a/tests/integration/image-tool.test.ts +++ b/tests/integration/image-tool.test.ts @@ -109,7 +109,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { // The CLI should be called with the DIRECTORY, not a full file path expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", MOCK_TEMP_DIR]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); // Verify the result is correct @@ -149,7 +150,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { expect(mockResolveImagePath).toHaveBeenCalledWith({}, mockContext.logger); expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", MOCK_TEMP_DIR]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); }); @@ -201,7 +203,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { expect(result.isError).toBeFalsy(); expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["image", "--mode", "screen", "--screen-index", "0"]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); // Since temp dir was used, saved_files now contains the temp file const mockResponse = mockSwiftCli.captureImage("screen", { @@ -239,7 +242,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { ); expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.not.arrayContaining(["--screen-index"]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -272,7 +276,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { expect(result.isError).toBeFalsy(); expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["image", "--mode", "screen", "--screen-index", "99"]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); // Since temp dir was used, saved_files now contains the temp file expect(result.saved_files).toEqual([{ @@ -900,7 +905,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { // It should have used the default path expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", MOCK_DEFAULT_PATH]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); // No cleanup should have occurred @@ -942,7 +948,8 @@ describeSwiftTests("Image Tool Integration Tests", () => { // We can verify this by checking that the Swift CLI was called with the temp dir, not the default path expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", MOCK_TEMP_DIR]), - mockContext.logger + mockContext.logger, + expect.objectContaining({ timeout: expect.any(Number) }) ); delete process.env.PEEKABOO_DEFAULT_SAVE_PATH; diff --git a/tests/unit/tools/image-edge-cases.test.ts b/tests/unit/tools/image-edge-cases.test.ts index edbef2d..eaf1395 100644 --- a/tests/unit/tools/image-edge-cases.test.ts +++ b/tests/unit/tools/image-edge-cases.test.ts @@ -103,6 +103,7 @@ describe("Image Tool - Edge Cases", () => { "--window-title", "Apple" ]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); }); @@ -128,6 +129,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -151,6 +153,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "jpg"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -174,6 +177,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "jpg"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -197,6 +201,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "jpg"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -222,6 +227,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); }); @@ -369,6 +375,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", pathWithPipe]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -393,6 +400,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", pathWithColon]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -417,6 +425,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", pathWithAsterisk]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -441,6 +450,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", complexPath]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -465,6 +475,7 @@ describe("Image Tool - Edge Cases", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", pathWithSpaces]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); }); diff --git a/tests/unit/tools/image.test.ts b/tests/unit/tools/image.test.ts index 9dcf745..bcd2aa2 100644 --- a/tests/unit/tools/image.test.ts +++ b/tests/unit/tools/image.test.ts @@ -92,6 +92,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["image", "--mode", "screen", "--path", MOCK_TEMP_IMAGE_DIR, "--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); // When format is omitted, it defaults to "png", not "data" @@ -139,6 +140,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); // Should NOT return base64 data for screen captures @@ -205,6 +207,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", userPath, "--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); expect(result.content).toEqual( expect.arrayContaining([ @@ -283,6 +286,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--mode", "screen", "--screen-index", "1"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -310,10 +314,12 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--mode", "screen"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.not.arrayContaining(["--screen-index"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -343,6 +349,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -372,6 +379,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "jpg"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -398,6 +406,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--mode", "screen"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -419,6 +428,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--app", "Safari", "--mode", "multi"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -444,6 +454,7 @@ describe("Image Tool", () => { "--window-title", "Apple" ]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -469,6 +480,7 @@ describe("Image Tool", () => { "--window-index", "2" ]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -490,6 +502,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--capture-focus", "foreground"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -511,6 +524,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--capture-focus", "auto"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -532,6 +546,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--capture-focus", "background"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); }); @@ -574,6 +589,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", MOCK_TEMP_IMAGE_DIR]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); expect(mockPerformAutomaticAnalysis).toHaveBeenCalledWith( "base64dataforanalysis", @@ -633,6 +649,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--path", USER_PATH, "--format", "jpg"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); expect(mockPerformAutomaticAnalysis).toHaveBeenCalledWith( "base64dataforanalysis", @@ -1170,6 +1187,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); @@ -1201,6 +1219,7 @@ describe("Image Tool", () => { expect(mockExecuteSwiftCli).toHaveBeenCalledWith( expect.arrayContaining(["--format", "png"]), mockLogger, + expect.objectContaining({ timeout: expect.any(Number) }) ); }); });