mirror of
https://github.com/samsonjs/Peekaboo.git
synced 2026-03-25 09:25:47 +00:00
fix: Handle edge cases for invalid screen index and JSON null paths
- Invalid screen index (e.g., screen:99) now properly falls back to capturing all screens with unique filenames - String "null" in path parameter is now correctly treated as undefined instead of literal path - Added fallback-aware filename generation to prevent file overwrites when screen index is out of bounds - Comprehensive test coverage for both edge cases 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
000da1e2c1
commit
089d96ce22
3 changed files with 197 additions and 2 deletions
|
|
@ -213,7 +213,8 @@ struct ImageCommand: ParsableCommand {
|
|||
return try [captureSingleDisplay(displayID: displayID, index: screenIndex, labelSuffix: labelSuffix)]
|
||||
} else {
|
||||
Logger.shared.debug("Screen index \(screenIndex) is out of bounds. Capturing all screens instead.")
|
||||
return try captureAllScreens(displays: displays)
|
||||
// When falling back to all screens, use fallback-aware capture to prevent filename conflicts
|
||||
return try captureAllScreensWithFallback(displays: displays)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -225,6 +226,15 @@ struct ImageCommand: ParsableCommand {
|
|||
}
|
||||
return savedFiles
|
||||
}
|
||||
|
||||
private func captureAllScreensWithFallback(displays: [CGDirectDisplayID]) throws(CaptureError) -> [SavedFile] {
|
||||
var savedFiles: [SavedFile] = []
|
||||
for (index, displayID) in displays.enumerated() {
|
||||
let savedFile = try captureSingleDisplayWithFallback(displayID: displayID, index: index, labelSuffix: "")
|
||||
savedFiles.append(savedFile)
|
||||
}
|
||||
return savedFiles
|
||||
}
|
||||
|
||||
private func captureSingleDisplay(
|
||||
displayID: CGDirectDisplayID,
|
||||
|
|
@ -245,6 +255,26 @@ struct ImageCommand: ParsableCommand {
|
|||
mime_type: format == .png ? "image/png" : "image/jpeg"
|
||||
)
|
||||
}
|
||||
|
||||
private func captureSingleDisplayWithFallback(
|
||||
displayID: CGDirectDisplayID,
|
||||
index: Int,
|
||||
labelSuffix: String
|
||||
) throws(CaptureError) -> SavedFile {
|
||||
let fileName = generateFileName(displayIndex: index)
|
||||
let filePath = getOutputPathWithFallback(fileName)
|
||||
|
||||
try captureDisplay(displayID, to: filePath)
|
||||
|
||||
return SavedFile(
|
||||
path: filePath,
|
||||
item_label: "Display \(index + 1)\(labelSuffix)",
|
||||
window_title: nil,
|
||||
window_id: nil,
|
||||
window_index: nil,
|
||||
mime_type: format == .png ? "image/png" : "image/jpeg"
|
||||
)
|
||||
}
|
||||
|
||||
private func captureApplicationWindow(_ appIdentifier: String) throws -> [SavedFile] {
|
||||
let targetApp: NSRunningApplication
|
||||
|
|
@ -590,6 +620,14 @@ struct ImageCommand: ParsableCommand {
|
|||
"/tmp/\(fileName)"
|
||||
}
|
||||
}
|
||||
|
||||
func getOutputPathWithFallback(_ fileName: String) -> String {
|
||||
if let basePath = path {
|
||||
determineOutputPathWithFallback(basePath: basePath, fileName: fileName)
|
||||
} else {
|
||||
"/tmp/\(fileName)"
|
||||
}
|
||||
}
|
||||
|
||||
func determineOutputPath(basePath: String, fileName: String) -> String {
|
||||
// Check if basePath looks like a file (has extension and doesn't end with /)
|
||||
|
|
@ -642,6 +680,54 @@ struct ImageCommand: ParsableCommand {
|
|||
return "\(basePath)/\(fileName)"
|
||||
}
|
||||
}
|
||||
|
||||
func determineOutputPathWithFallback(basePath: String, fileName: String) -> String {
|
||||
// Check if basePath looks like a file (has extension and doesn't end with /)
|
||||
// Exclude special directory cases like "." and ".."
|
||||
let isLikelyFile = basePath.contains(".") && !basePath.hasSuffix("/") &&
|
||||
basePath != "." && basePath != ".."
|
||||
|
||||
if isLikelyFile {
|
||||
// Create parent directory if needed
|
||||
let parentDir = (basePath as NSString).deletingLastPathComponent
|
||||
if !parentDir.isEmpty && parentDir != "/" {
|
||||
do {
|
||||
try FileManager.default.createDirectory(
|
||||
atPath: parentDir,
|
||||
withIntermediateDirectories: true,
|
||||
attributes: nil
|
||||
)
|
||||
} catch {
|
||||
// Log but don't fail - maybe directory already exists
|
||||
// Logger.debug("Could not create parent directory \(parentDir): \(error)")
|
||||
}
|
||||
}
|
||||
|
||||
// For fallback mode (invalid screen index that fell back to all screens),
|
||||
// always treat as multiple screens to avoid overwriting
|
||||
let pathExtension = (basePath as NSString).pathExtension
|
||||
let pathWithoutExtension = (basePath as NSString).deletingPathExtension
|
||||
|
||||
// Extract screen info from fileName (e.g., "screen_1_20250608_120000.png" -> "1_20250608_120000")
|
||||
let fileNameWithoutExt = (fileName as NSString).deletingPathExtension
|
||||
let screenSuffix = fileNameWithoutExt.replacingOccurrences(of: "screen_", with: "")
|
||||
|
||||
return "\(pathWithoutExtension)_\(screenSuffix).\(pathExtension)"
|
||||
} else {
|
||||
// Treat as directory - ensure it exists
|
||||
do {
|
||||
try FileManager.default.createDirectory(
|
||||
atPath: basePath,
|
||||
withIntermediateDirectories: true,
|
||||
attributes: nil
|
||||
)
|
||||
} catch {
|
||||
// Log but don't fail - maybe directory already exists
|
||||
// Logger.debug("Could not create directory \(basePath): \(error)")
|
||||
}
|
||||
return "\(basePath)/\(fileName)"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
extension DateFormatter {
|
||||
|
|
|
|||
|
|
@ -133,7 +133,7 @@ export const imageToolSchema = z.object({
|
|||
}
|
||||
return val;
|
||||
},
|
||||
z.string().optional()
|
||||
z.string().optional(),
|
||||
).describe(
|
||||
"Optional. Base absolute path for saving the image.\n" +
|
||||
"Relevant if `format` is `'png'`, `'jpg'`, or if `'data'` is used with the intention to also save the file.\n" +
|
||||
|
|
|
|||
109
tests/unit/tools/edge-case-fixes.test.ts
Normal file
109
tests/unit/tools/edge-case-fixes.test.ts
Normal file
|
|
@ -0,0 +1,109 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { imageToolSchema } from "../../../src/types/index";
|
||||
|
||||
describe("Edge Case Fixes", () => {
|
||||
describe("JSON null string handling", () => {
|
||||
it("should treat string 'null' as undefined path", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
path: "null" // JSON string "null" should be treated as undefined
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
// String "null" should be preprocessed to undefined
|
||||
expect(result.path).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should handle actual null values correctly", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
path: null
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
// Actual null should also become undefined
|
||||
expect(result.path).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should handle empty string correctly", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
path: ""
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
// Empty string should also become undefined
|
||||
expect(result.path).toBeUndefined();
|
||||
});
|
||||
|
||||
it("should preserve valid path strings", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
path: "/tmp/test.png"
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
// Valid path should be preserved
|
||||
expect(result.path).toBe("/tmp/test.png");
|
||||
});
|
||||
});
|
||||
|
||||
describe("Invalid screen index edge cases", () => {
|
||||
it("should handle app_target with invalid screen index", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
app_target: "screen:99"
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
// Should parse correctly - invalid index handling is done in Swift CLI
|
||||
expect(result.app_target).toBe("screen:99");
|
||||
expect(result.format).toBe("png");
|
||||
});
|
||||
|
||||
it("should handle app_target with negative screen index", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
app_target: "screen:-1"
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
expect(result.app_target).toBe("screen:-1");
|
||||
expect(result.format).toBe("png");
|
||||
});
|
||||
|
||||
it("should handle app_target with non-numeric screen index", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
app_target: "screen:abc"
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
expect(result.app_target).toBe("screen:abc");
|
||||
expect(result.format).toBe("png");
|
||||
});
|
||||
});
|
||||
|
||||
describe("Combined edge cases", () => {
|
||||
it("should handle both null path and invalid screen index", () => {
|
||||
const input = {
|
||||
format: "png",
|
||||
app_target: "screen:99",
|
||||
path: "null"
|
||||
};
|
||||
|
||||
const result = imageToolSchema.parse(input);
|
||||
|
||||
expect(result.app_target).toBe("screen:99");
|
||||
expect(result.path).toBeUndefined();
|
||||
expect(result.format).toBe("png");
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue