-
Notifications
You must be signed in to change notification settings - Fork 105
feat: add open working directory button to header #8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a new POST API route at Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Header as Header (React)
participant API as /api/open-directory
participant OS as Operating System
User->>Header: Click "Open Project Folder"
activate Header
Header->>Header: if saveDirectoryPath present
alt path exists
Header->>API: POST /api/open-directory\n{ "path": "<saveDirectoryPath>" }
activate API
API->>API: parse & validate path
API->>API: detect platform\n(select command)
API->>OS: execute command\n(open / explorer / xdg-open)
activate OS
OS-->>API: execution result
deactivate OS
API-->>Header: { success: true } / { success: false, error }
deactivate API
Header->>User: show success or alert on error
else missing path
Header-->>User: no-op / early return
end
deactivate Header
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/app/api/open-directory/route.ts (2)
39-41: Consider validating path existence before executing the command.Adding a check to verify the path exists and is a directory would provide better error messages to users and prevent unnecessary command execution attempts.
🔎 Suggested enhancement
import { NextRequest, NextResponse } from "next/server"; import { exec } from "child_process"; import { promisify } from "util"; import os from "os"; +import { existsSync, statSync } from "fs"; const execAsync = promisify(exec); export async function POST(req: NextRequest) { try { const body = await req.json(); const { path } = body; if (!path) { return NextResponse.json( { success: false, error: "Path is required" }, { status: 400 } ); } + // Validate path exists and is a directory + if (!existsSync(path)) { + return NextResponse.json( + { success: false, error: "Directory does not exist" }, + { status: 400 } + ); + } + + if (!statSync(path).isDirectory()) { + return NextResponse.json( + { success: false, error: "Path is not a directory" }, + { status: 400 } + ); + } let command = ""; const platform = os.platform();
42-48: Consider more specific error handling.The current catch-all error handler returns a generic message. You could enhance user experience by distinguishing between different failure types (e.g., command not found, permission denied) and returning more specific error messages.
🔎 Optional enhancement
} catch (error) { console.error("Failed to open directory:", error); + + const errorMessage = error instanceof Error ? error.message : "Unknown error"; + let userMessage = "Failed to open directory"; + + // Provide more specific errors based on common failure modes + if (errorMessage.includes("ENOENT") || errorMessage.includes("not found")) { + userMessage = "Directory not found or command unavailable"; + } else if (errorMessage.includes("EACCES") || errorMessage.includes("permission")) { + userMessage = "Permission denied"; + } + return NextResponse.json( - { success: false, error: "Failed to open directory" }, + { success: false, error: userMessage }, { status: 500 } ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/api/open-directory/route.tssrc/components/Header.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Handles can only connect to handles of the same type (image→image,text→text); cross-type connections likeimage→textare invalid
Connections must flow fromsource(output) totarget(input) handles
Image inputs on generation nodes accept multiple connections (for multi-image context); text inputs accept single connections (last connected wins)
Use descriptive handle IDs that match the data type:id="image"for image data,id="text"for text data
Extract image data fromimageInputviadata.image, fromannotationviadata.outputImage, and fromnanoBananaviadata.outputImage
Extract text data frompromptviadata.promptand fromllmGenerateviadata.outputText
Files:
src/app/api/open-directory/route.tssrc/components/Header.tsx
🧬 Code graph analysis (1)
src/app/api/open-directory/route.ts (1)
src/app/api/browse-directory/route.ts (1)
GET(11-160)
🔇 Additional comments (1)
src/app/api/open-directory/route.ts (1)
14-19: Basic validation is correct.The presence check for the path parameter is appropriate and returns a proper 400 error response. However, consider adding additional validation to ensure the path is a valid directory path before attempting to open it (see comment on lines 24-37).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/app/api/open-directory/route.ts (1)
14-19: Add path validation and sanitization.The current implementation only checks if the path is truthy, without verifying that it exists, is a directory, or is within safe boundaries. This can lead to:
- Confusing error messages when invalid paths are provided
- Potential information disclosure through system error messages
- Failed operations that could have been caught earlier with better validation
🔎 Proposed fix to add comprehensive path validation
import { NextRequest, NextResponse } from "next/server"; import { execFile } from "child_process"; import { promisify } from "util"; import os from "os"; +import { existsSync, statSync, realpathSync } from "fs"; +import { resolve } from "path"; const execFileAsync = promisify(execFile); export async function POST(req: NextRequest) { try { const body = await req.json(); const { path } = body; if (!path) { return NextResponse.json( { success: false, error: "Path is required" }, { status: 400 } ); } + + // Validate path format and sanitize + if (typeof path !== "string" || path.includes("\0")) { + return NextResponse.json( + { success: false, error: "Invalid path format" }, + { status: 400 } + ); + } + + // Resolve and validate the path exists + let resolvedPath: string; + try { + resolvedPath = realpathSync(resolve(path)); + } catch { + return NextResponse.json( + { success: false, error: "Path does not exist" }, + { status: 400 } + ); + } + + // Verify it's a directory + try { + if (!statSync(resolvedPath).isDirectory()) { + return NextResponse.json( + { success: false, error: "Path is not a directory" }, + { status: 400 } + ); + } + } catch { + return NextResponse.json( + { success: false, error: "Cannot access path" }, + { status: 400 } + ); + } let command = ""; let args: string[] = []; const platform = os.platform(); switch (platform) { case "darwin": command = "open"; - args = [path]; + args = [resolvedPath]; break; case "win32": command = "explorer"; - args = [path]; + args = [resolvedPath]; break; case "linux": command = "xdg-open"; - args = [path]; + args = [resolvedPath]; break; default: // Fallback for other Unix-like systems command = "xdg-open"; - args = [path]; + args = [resolvedPath]; } await execFileAsync(command, args); return NextResponse.json({ success: true }); } catch (error) { console.error("Failed to open directory:", error); return NextResponse.json( { success: false, error: "Failed to open directory" }, { status: 500 } ); } }
🧹 Nitpick comments (1)
src/app/api/open-directory/route.ts (1)
21-46: Consider adding timeout and improving error specificity.The platform detection and command execution logic is sound and secure. However, there are a couple of optional improvements that could enhance reliability:
- The
execFileAsynccall has no timeout, which could cause the request to hang indefinitely if the system command doesn't respond.- The
commandvariable is initialized to an empty string but TypeScript expects it to always have a value in the switch cases.🔎 Suggested enhancements
- let command = ""; + let command: string; let args: string[] = []; const platform = os.platform(); switch (platform) { case "darwin": command = "open"; args = [path]; break; case "win32": command = "explorer"; args = [path]; break; case "linux": command = "xdg-open"; args = [path]; break; default: // Fallback for other Unix-like systems command = "xdg-open"; args = [path]; } - await execFileAsync(command, args); + await execFileAsync(command, args, { timeout: 5000 }); return NextResponse.json({ success: true });The 5-second timeout prevents the API from hanging if the system command is unresponsive, improving overall reliability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/api/open-directory/route.tssrc/components/Header.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Handles can only connect to handles of the same type (image→image,text→text); cross-type connections likeimage→textare invalid
Connections must flow fromsource(output) totarget(input) handles
Image inputs on generation nodes accept multiple connections (for multi-image context); text inputs accept single connections (last connected wins)
Use descriptive handle IDs that match the data type:id="image"for image data,id="text"for text data
Extract image data fromimageInputviadata.image, fromannotationviadata.outputImage, and fromnanoBananaviadata.outputImage
Extract text data frompromptviadata.promptand fromllmGenerateviadata.outputText
Files:
src/components/Header.tsxsrc/app/api/open-directory/route.ts
🧬 Code graph analysis (1)
src/app/api/open-directory/route.ts (2)
src/app/api/browse-directory/route.ts (1)
GET(11-160)src/components/ProjectSetupModal.tsx (1)
target(42-72)
🔇 Additional comments (3)
src/app/api/open-directory/route.ts (1)
1-7: LGTM! Proper use ofexecFilefor security.The imports correctly use
execFileinstead ofexec, which prevents shell injection vulnerabilities. The promisify pattern is appropriate for async/await usage.src/components/Header.tsx (2)
80-103: LGTM! All previous feedback addressed.The function properly validates responses, provides user feedback, and handles errors gracefully. The early return guard ensures the function only executes when
saveDirectoryPathis available.
154-174: LGTM! Button visibility issue resolved.The conditional rendering ensures the button only appears when
saveDirectoryPathexists, preventing the silent failure scenario flagged in the previous review. The button integrates cleanly with the existing header layout and styling.
Summary
This PR adds a new button to the application header that allows users to instantly open the current project's working directory in their system's file explorer.
Motivation
Currently, users must leave the application and manually navigate to the project folder to view generated assets or manage files. This feature streamlines the workflow.
Changes
src/app/api/open-directory/route.tswhich handles the command to open the folder (supportsxdg-openon Linux,openon macOS, andexploreron Windows).execFileand strictly validates inputs to prevent command injection.Header.tsxto include a folder icon button.Verification
execFilewith args).