fix: Windows compatibility, PATH handling, and SSH execution improvements#268
fix: Windows compatibility, PATH handling, and SSH execution improvements#268pedramamini merged 5 commits into0.15.0-rcfrom
Conversation
- Fix locale formatting issues in AgentSessionsModal and AgentPromptComposerModal tests - Update path normalization in stats-db tests for Windows compatibility - Correct mock signatures in AutoRun and agent-spawner tests - Make agent detection tests platform-aware (powershell.exe vs bash) - Fix PATH expansion test to handle platform-specific delimiters - Ensure consistent 'en-US' locale usage across components and tests All tests now pass on both Windows and Unix systems.
- Fix Windows agent execution by using buildExpandedEnv for proper PATH expansion - Add PowerShell support for SSH commands to handle long command lines (32K+ chars) - Implement here document support for large OpenCode prompts over SSH - Add raw stdin prompt sending for agents without stream-json support - Restrict inline wizard to Claude, Claude Code, and Codex (OpenCode incompatible) - Improve argument escaping for both cmd.exe and PowerShell shells - Update tsconfig.main.json to include shared files for proper compilation - Enhance agent path resolution for packaged Electron applications - Add read-only mode for OpenCode in wizard conversations - Update tests and UI components for better SSH remote configuration
- Fix CLI storage path handling: use path.join instead of path.posix.join for proper Windows path construction on local filesystem operations - Fix streamJsonBuilder format: restore correct Claude Code stream-json format with type: 'user' and nested message structure (was incorrectly changed to type: 'user_message' which causes Claude CLI errors) - Add shellEscape utility: create proper shell argument escaping module with documentation and comprehensive tests for cmd.exe and PowerShell - Refactor ChildProcessSpawner: use new shellEscape utility instead of inline escaping logic for better maintainability - Fix type errors: correct ToolType usage (remove invalid 'claude' value) and add explicit Record<string, AgentSshRemoteConfig> typing
Add a new preview panel that opens within the Document Graph view when pressing Enter on a focused node. This allows exploring markdown files without leaving the graph view. Changes: - Enter key now opens in-graph preview panel (double-click to recenter) - Preview panel slides in from the right with full markdown rendering - Escape closes the preview and returns focus to the graph for navigation - Preview content is focusable for keyboard scrolling - Updated Help panel to reflect new keyboard shortcuts
- Added IPC/API to fetch earliest recorded stats timestamp instantly 🧭 - Stats DB now computes earliest date across key tracking tables accurately 🧠 - Settings modal displays “since YYYY-MM-DD” alongside stats database size 📅
Pull Request Review: Windows Compatibility and SSH Execution ImprovementsThis is a comprehensive PR addressing Windows compatibility, PATH handling, and SSH remote execution improvements. Changes span 67 files with 1,588 additions and 683 deletions. Strengths1. Excellent Shell Escaping ImplementationThe new shellEscape.ts module is well-designed with comprehensive test coverage (140 lines), clear documentation with Microsoft references, and proper separation of cmd.exe vs PowerShell escaping logic. 2. Critical Bug Fixes
3. Comprehensive Test Platform AwarenessAll tests properly updated to be platform-aware using path.delimiter, path.normalize(), and platform-specific expectations. Critical Issues1. Remaining Bug in storage.ts:440The legacy history write path STILL uses path.posix.join instead of path.join: const filePath = path.posix.join(getConfigDir(), 'maestro-history.json'); // ❌ Should be path.joinThis needs to be fixed before merge. 2. Security: Shell Escaping Edge CasesIn ChildProcessSpawner.ts:184, escapeArgsForShell escapes each argument individually, but when spawning with shell: true, arguments are concatenated which could lead to shell injection vulnerabilities. Recommendation: Add security testing for arguments containing shell metacharacters like &, |, ;, etc. 3. Type Safety Issue (process.ts:43-44)Using Record<string, any> defeats TypeScript type safety. Should define proper AgentConfig interface. Important Issues4. Console.log in Production CodeChildProcessSpawner.ts:331-337 uses direct console.log instead of the logger. Should use logger.debug for consistency. 5. Large Prompt Logging SecurityWindows logging could expose sensitive information in prompts. Consider adding verbose flag and redacting sensitive patterns (API keys, tokens). Recommendations SummaryCritical (Must Fix Before Merge):
Important (Should Fix): Nice to Have: ConclusionThis is high-quality work with excellent shell escaping implementation and comprehensive test coverage. The main concerns are the remaining path.posix.join bug, potential shell injection vectors, and type safety. Recommendation: Request changes to address critical issues above, then approve after fixes. The work is solid and moves Maestro toward better Windows support. Great work overall! Reviewed using repository CLAUDE.md for context and conventions. |
2 participants
Summary
This PR contains Windows compatibility fixes, PATH handling improvements, and SSH execution enhancements for the 0.15.0 release.
Changes
buildExpandedPath,buildExpandedEnv)path.joininstead ofpath.posix.joinfor Windows compatibilitytype: 'user'structureTesting