feat: add Xcode scheme discovery API endpoint #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added support for discovering Xcode project schemes, targets, and configurations using xcodebuild. The implementation includes a new API endpoint that accepts either direct paths to .xcodeproj/.xcworkspace files or directory paths, automatically finding and prioritizing workspace files over projects. The discovery logic uses xcodebuild -list -json to extract project metadata and includes comprehensive test coverage for all edge cases including nonexistent paths, directories without projects, and workspace priority handling.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new API endpoint /api/xcode/schemes for discovering Xcode project metadata including schemes, targets, and configurations. The implementation uses xcodebuild -list -json to extract project information and supports both direct file paths (.xcodeproj, .xcworkspace) and directory paths with automatic discovery, prioritizing workspace files over projects.
Key changes:
- New
xcodemodule with project discovery functionality - POST endpoint at
/api/xcode/schemesaccepting a JSON path parameter - Comprehensive unit and integration test coverage for various scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/xcode/schemes.rs | Core implementation of Xcode project discovery using xcodebuild, including path resolution logic and JSON parsing |
| app/src/xcode/mod.rs | Module declaration and re-exports for the xcode module |
| app/src/routes/mod.rs | New POST endpoint handler for retrieving Xcode schemes |
| app/src/lib.rs | Made modules public to support integration testing |
| app/tests/xcode_integration_tests.rs | Integration tests covering endpoint behavior for various edge cases including nonexistent paths, directories without projects, and workspace priority |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
app/src/xcode/schemes.rs
Outdated
| } | ||
|
|
||
| // Otherwise, search in the directory | ||
| let search_dir = if path.is_dir() { path } else { path.parent().unwrap() }; |
There was a problem hiding this comment.
The unwrap() call here could panic if the path has no parent (e.g., if path is the root directory "/" or a path component without a parent). This should be handled gracefully since the path could be user-provided. Consider returning an error instead.
Suggested change
| let search_dir = if path.is_dir() { path } else { path.parent().unwrap() }; | |
| let search_dir = if path.is_dir() { | |
| path | |
| } else { | |
| path | |
| .parent() | |
| .ok_or_else(|| format!("Path has no parent directory: {}", path.display()))? | |
| }; |
app/src/xcode/schemes.rs
Outdated
Comment on lines
97
to
116
| // Prefer workspace over project | ||
| for entry in std::fs::read_dir(search_dir).map_err(|e| format!("Failed to read directory: {}", e))? { | ||
| let entry = entry.map_err(|e| format!("Failed to read entry: {}", e))?; | ||
| let entry_path = entry.path(); | ||
|
|
||
| if let Some(ext) = entry_path.extension() { | ||
| if ext == "xcworkspace" { | ||
| return Ok((entry_path, ProjectType::Workspace)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Fall back to project if no workspace found | ||
| for entry in std::fs::read_dir(search_dir).map_err(|e| format!("Failed to read directory: {}", e))? { | ||
| let entry = entry.map_err(|e| format!("Failed to read entry: {}", e))?; | ||
| let entry_path = entry.path(); | ||
|
|
||
| if let Some(ext) = entry_path.extension() { | ||
| if ext == "xcodeproj" { | ||
| return Ok((entry_path, ProjectType::Project)); |
There was a problem hiding this comment.
The directory is being read twice - once to search for workspace files (lines 98-107) and again to search for project files (lines 110-119). This is inefficient and could cause issues if the directory contents change between reads. Consider collecting all entries in a single pass and then checking for workspace files first, then project files.
Suggested change
| // Prefer workspace over project | |
| for entry in std::fs::read_dir(search_dir).map_err(|e| format!("Failed to read directory: {}", e))? { | |
| let entry = entry.map_err(|e| format!("Failed to read entry: {}", e))?; | |
| let entry_path = entry.path(); | |
| if let Some(ext) = entry_path.extension() { | |
| if ext == "xcworkspace" { | |
| return Ok((entry_path, ProjectType::Workspace)); | |
| } | |
| } | |
| } | |
| // Fall back to project if no workspace found | |
| for entry in std::fs::read_dir(search_dir).map_err(|e| format!("Failed to read directory: {}", e))? { | |
| let entry = entry.map_err(|e| format!("Failed to read entry: {}", e))?; | |
| let entry_path = entry.path(); | |
| if let Some(ext) = entry_path.extension() { | |
| if ext == "xcodeproj" { | |
| return Ok((entry_path, ProjectType::Project)); | |
| // Read directory entries once | |
| let entries: Vec<PathBuf> = std::fs::read_dir(search_dir) | |
| .map_err(|e| format!("Failed to read directory: {}", e))? | |
| .map(|entry_res| { | |
| entry_res | |
| .map(|entry| entry.path()) | |
| .map_err(|e| format!("Failed to read entry: {}", e)) | |
| }) | |
| .collect::<Result<_, _>>()?; | |
| // Prefer workspace over project | |
| for entry_path in &entries { | |
| if let Some(ext) = entry_path.extension() { | |
| if ext == "xcworkspace" { | |
| return Ok((entry_path.clone(), ProjectType::Workspace)); | |
| } | |
| } | |
| } | |
| // Fall back to project if no workspace found | |
| for entry_path in &entries { | |
| if let Some(ext) = entry_path.extension() { | |
| if ext == "xcodeproj" { | |
| return Ok((entry_path.clone(), ProjectType::Project)); |
app/src/routes/mod.rs
Outdated
| async fn get_xcode_schemes(Json(request): Json<XcodeSchemesRequest>) -> impl IntoResponse { | ||
| let path = Path::new(&request.path); | ||
|
|
||
| match xcode::discover_project(path) { |
There was a problem hiding this comment.
The discover_project function performs blocking I/O operations (file system reads, spawning a process, waiting for output) but is being called directly in an async handler. This can block the Tokio runtime thread pool and degrade performance. Consider wrapping the call in tokio::task::spawn_blocking to move the blocking work to a dedicated thread pool.
Refactored the routes module from a single 521-line file into focused, maintainable modules: - health.rs: health check and about endpoints - projects.rs: project validation and recent projects logic - xcode.rs: Xcode scheme discovery endpoint - mod.rs: module declarations and router setup (now just 31 lines) Also applied cargo fmt to fix formatting issues. All tests still pass (22 unit tests + 7 integration tests).
- Fix potential panic in find_project_or_workspace by handling missing parent gracefully - Optimize directory reading by collecting entries once instead of twice - Use spawn_blocking for xcodebuild calls to avoid blocking async runtime All integration tests still passing.
Added two new integration tests that use a real Xcode project fixture: - test_real_xcode_project_discovery: Tests discovery via direct project path - test_real_xcode_project_discovery_from_directory: Tests discovery via directory path The fixture is a minimal but valid Xcode project with: - 1 scheme: "Plasma Project" - 1 target: "Plasma" - 2 configurations: Debug and Release This provides more comprehensive testing than the mock fixtures used in other tests.
Replaced blocking I/O with async operations: - Changed std::process::Command to tokio::process::Command for running xcodebuild - Changed std::fs to tokio::fs for directory reading and metadata checks - Removed spawn_blocking wrapper as all operations are now async - Updated all unit tests to use #[tokio::test] This provides better async runtime integration without blocking worker threads.
- Added #[serde(rename_all = "lowercase")] to ProjectType enum - Removed manual match for enum-to-string conversion in route handler - Enum now serializes automatically via serde
Changed /api/xcode/schemes to /api/xcode/discover since the endpoint returns more than just schemes (also targets and configurations). - Renamed XcodeSchemesRequest to DiscoverProjectRequest - Renamed get_xcode_schemes handler to discover_project - Updated route from /xcode/schemes to /xcode/discover - Updated all integration tests to use new endpoint path
Extracted business logic from route handlers into a dedicated services layer. Route handlers now serve as thin HTTP mappers that delegate to the services layer and map results to HTTP responses. This improves separation of concerns and makes the code more maintainable and testable. Changes: - Created src/services/projects.rs with all project detection and database logic - Simplified src/routes/projects.rs to be a thin HTTP layer (99 lines vs 456) - Added services module to lib.rs - Removed unused PathBuf import
Refactored xcode module to reuse project detection logic from the services layer instead of duplicating it. The xcode module now focuses solely on Xcode-specific functionality (running xcodebuild to discover schemes, targets, and configurations). Changes: - Renamed src/xcode/schemes.rs to src/xcode/discovery.rs (better reflects its purpose) - Removed duplicate project detection logic from xcode module - Xcode discovery now delegates to services/projects.rs for project detection - Re-exported ProjectType from services module for visibility - Updated tests to handle both old and new error messages - All 16 tests passing
Replaced stringly-typed errors with a proper error enum using thiserror. This provides better type safety, enables pattern matching on error types, and makes error handling more idiomatic. Benefits: - Type-safe error handling with DiscoveryError enum - Automatic error conversion using #[from] for io::Error and serde_json::Error - Clear, descriptive error variants (ProjectNotFound, NotXcodeProject, etc.) - Better error messages and error context - Implements Display trait automatically via thiserror All 16 tests passing.
- Remove useless Value conversion in xcode route handler - Remove unused Value import - Remove needless return statement in lib.rs
Apply cargo fmt's preferred formatting for match arm
The tests that use real Xcode projects require xcodebuild, which is only available on macOS. Mark them with cfg(target_os = "macos") to skip on Linux CI.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for discovering Xcode project schemes, targets, and configurations. The new API endpoint accepts either direct paths to .xcodeproj or .xcworkspace files, or directory paths where it will automatically search for and prioritize workspace files over projects.
The implementation uses xcodebuild -list -json to extract project metadata and includes comprehensive test coverage for all edge cases including nonexistent paths, directories without projects, and workspace priority handling.