Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the DDEV plugin to improve path handling across all commands (not just cd commands). The refactor replaces the previous status-checking approach with a more robust caching system, introduces comprehensive path cleaning that converts host paths to container paths, and streamlines the DDEV availability detection interface.
- Replaced
DdevStatuswithDdevCachestructure that caches raw DDEV project data with timestamp-based invalidation - Introduced
cleanCommand()function that converts host paths to container-relative paths for all commands (previously only handledcd) - Added helper functions (
isAvailable(),isRunning(),getProjectRoot(),getContainerWorkingDir()) for cleaner state management
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ddev.ts
Outdated
| const containerWorkingDir = getContainerWorkingDir(); | ||
| const escapedCommand = JSON.stringify(cleanedCommand); | ||
|
|
||
| return `ddev exec --dir="${containerWorkingDir}" bash -c ${escapedCommand}`; |
There was a problem hiding this comment.
The containerWorkingDir is interpolated directly into the command string without escaping on line 292. If the container working directory path contains special characters (e.g., quotes, spaces), this could break the generated ddev exec command.
While it's unlikely that DDEV container paths would contain such characters (they're typically under /var/www/html), consider using proper escaping or validation for robustness. For example:
return `ddev exec --dir=${JSON.stringify(containerWorkingDir)} bash -c ${escapedCommand}`;Or add validation to ensure containerWorkingDir doesn't contain problematic characters.
| return `ddev exec --dir="${containerWorkingDir}" bash -c ${escapedCommand}`; | |
| return `ddev exec --dir=${JSON.stringify(containerWorkingDir)} bash -c ${escapedCommand}`; |
ddev.ts
Outdated
|
|
||
| const output = result.stdout.toString(); | ||
| const { path: projectRoot, status: projectStatus } = extractProjectInfo(output); | ||
| const data = JSON.parse(output); |
There was a problem hiding this comment.
[nitpick] The JSON.parse(output) call on line 249 could throw an error if the output from ddev describe -j is malformed JSON. While this is caught by the try-catch block on lines 239-268, it would be helpful to have more specific error handling or logging to distinguish between parse errors and other errors.
Consider adding a specific try-catch around the JSON.parse to provide clearer error messages, e.g.:
let data;
try {
data = JSON.parse(output);
} catch (parseError) {
console.error(`Failed to parse DDEV JSON output: ${parseError instanceof Error ? parseError.message : String(parseError)}`);
ddevCache = null;
return;
}This would make debugging easier if DDEV's output format changes or becomes corrupted.
| const data = JSON.parse(output); | |
| let data; | |
| try { | |
| data = JSON.parse(output); | |
| } catch (parseError) { | |
| console.error(`Failed to parse DDEV JSON output: ${parseError instanceof Error ? parseError.message : String(parseError)}`); | |
| ddevCache = null; | |
| return; | |
| } |
… for error tracking
This pull request refactors the DDEV plugin implementation in
ddev.tsto improve command handling, caching, and project state detection. The changes replace the previous status-checking logic with a more robust caching system, introduce advanced command cleaning to ensure correct container execution, and streamline the interface for checking DDEV availability and running state.The main reason for this PR was that path were previously fixed only for
cdbash commands. Now all commands are processed. I came across this issue when the ai executedmkdirinside ddev, but with the full path. This is the solution.Caching and State Detection Improvements
DdevStatustype and status-checking logic with a newDdevCachestructure, which caches raw DDEV project data and only persists the cache when DDEV is running. Added helper functionsisAvailable()andisRunning()for more reliable state checks. [1] [2] [3]Command Cleaning and Path Handling
cleanCommand()function to convert host paths to container-relative paths, remove redundantcdcommands, and ensure commands are properly formatted for container execution. This replaces the previousremoveRedundantCd()logic and is used in the newwrapWithDdevExec()function. [1] [2]Container Working Directory Calculation
getContainerWorkingDir()to dynamically determine the correct container working directory based on the host directory and DDEV project root, improving path mapping for commands executed in the container. [1] [2]Plugin Initialization and Event Handling
Logging and Tool Registration
These changes make the DDEV plugin more robust and maintainable by improving state management, command handling, and container path mapping.