diff --git a/src/tools/image.ts b/src/tools/image.ts index 1e97513..3e2d4cb 100644 --- a/src/tools/image.ts +++ b/src/tools/image.ts @@ -45,9 +45,40 @@ export async function imageToolHandler( } }; } + + // If success is true, data should be present + if (!swiftResponse.data) { + logger.error('Swift CLI reported success but no data was returned.'); + return { + content: [{ + type: 'text', + text: 'Image capture failed: Invalid response from capture utility (no data).' + }], + isError: true, + _meta: { + backend_error_code: 'INVALID_RESPONSE_NO_DATA' + } + }; + } const data = swiftResponse.data as ImageCaptureData; const content: any[] = []; + + // Check for errors related to the user-specified path if provided + // The original logic relied on savedFile.error, which doesn't exist. + // If the CLI fails to save to a user-specified path, it should ideally be reflected + // in swiftResponse.success or swiftResponse.messages. For now, this specific check is removed. + /* + if (input.path && data.saved_files?.length > 0) { + for (const savedFile of data.saved_files) { + if (savedFile.path.startsWith(input.path) && savedFile.error) { // savedFile.error is the issue + userPathSaveError = `Peekaboo Warning: Failed to save to user-specified path '${input.path}'. CLI Error: ${savedFile.error}`; + logger.warn({ userPath: input.path, fileError: savedFile.error }, 'Error saving to user-specified path'); + break; + } + } + } + */ // Add text summary const summary = generateImageCaptureSummary(data, input); @@ -55,10 +86,33 @@ export async function imageToolHandler( type: 'text', text: summary }); + + // If there was an error saving to the user-specified path, add it to the content + // This part is removed as userPathSaveError logic is removed. + /* + if (userPathSaveError) { + content.push({ + type: 'text', + text: userPathSaveError + }); + } + */ // Add image data if requested if (input.return_data && data.saved_files?.length > 0) { for (const savedFile of data.saved_files) { + // If swiftResponse.success is true, assume individual files were processed correctly by CLI + // unless specific messages indicate otherwise. The savedFile.error check is removed. + /* + if (savedFile.error) { // savedFile.error is the issue + logger.warn({ path: savedFile.path, error: savedFile.error }, 'Skipping base64 conversion for file with reported error by CLI'); + content.push({ + type: 'text', + text: `Peekaboo Warning: Could not process file '${savedFile.path}' due to CLI error: ${savedFile.error}` + }); + continue; + } + */ try { const imageBase64 = await readImageAsBase64(savedFile.path); content.push({ @@ -76,7 +130,7 @@ export async function imageToolHandler( logger.error({ error, path: savedFile.path }, 'Failed to read image file'); content.push({ type: 'text', - text: `Warning: Could not read image data from ${savedFile.path}` + text: `Warning: Could not read image data from ${savedFile.path}. Error: ${error instanceof Error ? error.message : 'Unknown error'}` }); } } @@ -92,7 +146,7 @@ export async function imageToolHandler( return { content, - saved_files: data.saved_files + saved_files: data.saved_files // Ensure this matches the expected return type, possibly ImageToolOutput }; } catch (error) { diff --git a/src/utils/peekaboo-cli.ts b/src/utils/peekaboo-cli.ts index e969214..8a2dbd1 100644 --- a/src/utils/peekaboo-cli.ts +++ b/src/utils/peekaboo-cli.ts @@ -94,12 +94,21 @@ export async function executeSwiftCli( if (exitCode !== 0 || !stdout.trim()) { logger.error({ exitCode, stdout, stderr }, 'Swift CLI execution failed'); + + // Prioritize stderr for the main message if available + const errorMessage = stderr.trim() + ? `Peekaboo CLI Error: ${stderr.trim()}` + : `Swift CLI execution failed (exit code: ${exitCode})`; + const errorDetails = stderr.trim() && stdout.trim() + ? `Stdout: ${stdout.trim()}` + : stderr.trim() ? '' : (stdout.trim() || 'No output received'); + resolve({ success: false, error: { - message: `Swift CLI execution failed (exit code: ${exitCode})`, + message: errorMessage, code: 'SWIFT_CLI_EXECUTION_ERROR', - details: stderr || stdout || 'No output received' + details: errorDetails } }); return; diff --git a/tests/unit/tools/image.test.ts b/tests/unit/tools/image.test.ts index dbf276e..9c840a3 100644 --- a/tests/unit/tools/image.test.ts +++ b/tests/unit/tools/image.test.ts @@ -176,10 +176,78 @@ describe('Image Tool', () => { expect(result.isError).toBeUndefined(); expect(result.content).toEqual(expect.arrayContaining([ expect.objectContaining({ type: 'text', text: expect.stringContaining('Captured 1 image') }), - expect.objectContaining({ type: 'text', text: 'Warning: Could not read image data from /tmp/fail.png' }) + expect.objectContaining({ type: 'text', text: 'Warning: Could not read image data from /tmp/fail.png. Error: Read failed' }) ])); expect(result.saved_files).toEqual([mockSavedFile]); }); + + it('should handle empty saved_files array', async () => { + const mockResponse = { + success: true, + data: { saved_files: [] } + }; + mockExecuteSwiftCli.mockResolvedValue(mockResponse); + + const result = await imageToolHandler({ + format: 'png', + return_data: false, + capture_focus: 'background' + }, mockContext); + + expect(result.content[0].text).toBe('Image capture completed but no files were saved.'); + expect(result.saved_files).toEqual([]); + }); + + it('should handle malformed Swift CLI response', async () => { + const mockResponse = { + success: true, + data: null // Invalid data, triggers the new check in imageToolHandler + }; + mockExecuteSwiftCli.mockResolvedValue(mockResponse as any); + + const result = await imageToolHandler({ + format: 'png', + return_data: false, + capture_focus: 'background' + }, mockContext); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('Image capture failed: Invalid response from capture utility (no data).'); + expect(result._meta?.backend_error_code).toBe('INVALID_RESPONSE_NO_DATA'); + }); + + it('should handle partial failures when reading multiple images', async () => { + const mockFiles: SavedFile[] = [ + { path: '/tmp/img1.png', mime_type: 'image/png', item_label: 'Image 1' }, + { path: '/tmp/img2.jpg', mime_type: 'image/jpeg', item_label: 'Image 2' } + ]; + const mockResponse = { + success: true, + data: { saved_files: mockFiles } + }; + mockExecuteSwiftCli.mockResolvedValue(mockResponse); + mockReadImageAsBase64 + .mockResolvedValueOnce('base64data1') + .mockRejectedValueOnce(new Error('Read failed')); + + const result = await imageToolHandler({ + format: 'png', + return_data: true, + capture_focus: 'background' + }, mockContext); + + expect(result.content).toEqual(expect.arrayContaining([ + expect.objectContaining({ type: 'text', text: expect.stringContaining('Captured 2 images')}), // Summary text + expect.objectContaining({ + type: 'image', + data: 'base64data1' + }), + expect.objectContaining({ + type: 'text', + text: 'Warning: Could not read image data from /tmp/img2.jpg. Error: Read failed' + }) + ])); + }); }); describe('buildSwiftCliArgs', () => { @@ -390,40 +458,6 @@ describe('Image Tool', () => { ); }); - it('should handle empty saved_files array', async () => { - const mockResponse = { - success: true, - data: { saved_files: [] } - }; - mockExecuteSwiftCli.mockResolvedValue(mockResponse); - - const result = await imageToolHandler({ - format: 'png', - return_data: false, - capture_focus: 'background' - }, mockContext); - - expect(result.content[0].text).toContain('Captured 0 images'); - expect(result.saved_files).toEqual([]); - }); - - it('should handle malformed Swift CLI response', async () => { - const mockResponse = { - success: true, - data: null // Invalid data - }; - mockExecuteSwiftCli.mockResolvedValue(mockResponse as any); - - const result = await imageToolHandler({ - format: 'png', - return_data: false, - capture_focus: 'background' - }, mockContext); - - expect(result.isError).toBe(true); - expect(result.content[0].text).toContain('Invalid response from image capture'); - }); - it('should handle multiple saved files', async () => { const mockFiles: SavedFile[] = [ { path: '/tmp/screen1.png', mime_type: 'image/png', item_label: 'Screen 1' }, @@ -484,38 +518,6 @@ describe('Image Tool', () => { expect(mockReadImageAsBase64).toHaveBeenCalledTimes(2); }); - it('should handle partial failures when reading multiple images', async () => { - const mockFiles: SavedFile[] = [ - { path: '/tmp/img1.png', mime_type: 'image/png', item_label: 'Image 1' }, - { path: '/tmp/img2.jpg', mime_type: 'image/jpeg', item_label: 'Image 2' } - ]; - const mockResponse = { - success: true, - data: { saved_files: mockFiles } - }; - mockExecuteSwiftCli.mockResolvedValue(mockResponse); - mockReadImageAsBase64 - .mockResolvedValueOnce('base64data1') - .mockRejectedValueOnce(new Error('Read failed')); - - const result = await imageToolHandler({ - format: 'png', - return_data: true, - capture_focus: 'background' - }, mockContext); - - expect(result.content).toEqual(expect.arrayContaining([ - expect.objectContaining({ - type: 'image', - data: 'base64data1' - }), - expect.objectContaining({ - type: 'text', - text: 'Warning: Could not read image data from /tmp/img2.jpg' - }) - ])); - }); - it('should handle Swift CLI timeout errors', async () => { const mockResponse = { success: false, @@ -534,7 +536,7 @@ describe('Image Tool', () => { expect(result.isError).toBe(true); expect(result.content[0].text).toContain('Command timed out'); - expect(result._meta.backend_error_code).toBe('TIMEOUT'); + expect(result._meta?.backend_error_code).toBe('TIMEOUT'); }); it('should handle window_specifier with both title and index', async () => {