-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Windows console popups during daemon spawn #1
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: main
Are you sure you want to change the base?
Conversation
Greptile SummaryThis PR fixes Windows console popups during daemon spawn and Chroma operations by using WMIC instead of Key Changes
Technical ApproachThe fix uses WMIC's Trade-offsDisabling Chroma on Windows means vector search functionality is unavailable on that platform until the MCP SDK subprocess spawning can be fixed or migrated to a persistent HTTP server approach. Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Plugin as Claude Code Plugin
participant PM as ProcessManager
participant WMIC as Windows WMIC
participant Worker as Worker Service
participant Settings as settings.json
Note over Plugin,Worker: Windows Daemon Spawn Flow
Plugin->>PM: spawnDaemon(scriptPath, port)
PM->>PM: Check if Windows platform
alt Windows Platform
PM->>PM: Build WMIC command<br/>(execPath + scriptPath + --daemon)
Note over PM: Environment variables NOT passed<br/>with WMIC (limitation)
PM->>WMIC: execSync("wmic process call create ...")
WMIC-->>PM: Return 0 (immediate)
Note over WMIC,Worker: Process spawned independently<br/>without console popup
WMIC->>Worker: Spawn worker process
Worker->>Settings: Read port from settings.json
Worker->>Worker: Start HTTP server on port
Worker->>PM: Write PID file after listen()
else Unix Platform
PM->>Worker: spawn() with detached: true
Note over PM,Worker: Environment variables passed<br/>via spawn options
Worker->>Worker: Read port from env or settings
Worker->>Worker: Start HTTP server
Worker->>PM: Write PID file after listen()
end
PM-->>Plugin: Return PID (0 on Windows)
Plugin->>Worker: Health check loop
Worker-->>Plugin: Ready
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
…operations Two changes to fix Windows Terminal popup issues: 1. Worker daemon spawn (ProcessManager.spawnDaemon): - Windows: Use WMIC to spawn truly independent process without console - WMIC creates processes that survive parent exit and have no console association - Properly handles paths with spaces via double-quoting - Unix: Unchanged behavior with standard detached spawn 2. PID file handling (worker-service.ts): - Worker now writes its own PID after listen() succeeds (all platforms) - Removes race condition where spawner wrote PID before worker was ready - On Windows, spawner PID was useless anyway 3. Chroma vector search (ChromaSync.ts): - Temporarily disabled on Windows to prevent MCP SDK subprocess popups - Will be re-enabled when we migrate to persistent HTTP server architecture - Windows users still get full observation storage, just no semantic search Tested on Windows 11 via SSH - worker spawns without console popups, survives parent process exit, and all lifecycle commands (start/stop/restart) work correctly. Fixes thedotmack#748, thedotmack#708, thedotmack#681, thedotmack#676, thedotmack#675 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ada1289 to
e171557
Compare
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.
4 files reviewed, 1 comment
| // Windows: Chroma disabled due to MCP SDK spawning console popups | ||
| // See: https://github.com/anthropics/claude-mem/issues/675 | ||
| // Will be re-enabled when we migrate to persistent HTTP server |
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.
syntax: Issue reference links to anthropics org instead of current repo
| // Windows: Chroma disabled due to MCP SDK spawning console popups | |
| // See: https://github.com/anthropics/claude-mem/issues/675 | |
| // Will be re-enabled when we migrate to persistent HTTP server | |
| // Windows: Chroma disabled due to MCP SDK spawning console popups | |
| // See: https://github.com/bigph00t/claude-mem/issues/675 | |
| // Will be re-enabled when we migrate to persistent HTTP server |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/sync/ChromaSync.ts
Line: 86:88
Comment:
**syntax:** Issue reference links to anthropics org instead of current repo
```suggestion
// Windows: Chroma disabled due to MCP SDK spawning console popups
// See: https://github.com/bigph00t/claude-mem/issues/675
// Will be re-enabled when we migrate to persistent HTTP server
```
How can I resolve this? If you propose a fix, please make it concise.Commands /do and /make-plan were not appearing in Claude Code because they lacked the required YAML frontmatter metadata. Added description and argument-hint fields to both commands. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
6 files reviewed, 1 comment
| const execPath = process.execPath; | ||
| const script = scriptPath; | ||
| // WMIC command format: wmic process call create "\"path1\" \"path2\" args" | ||
| const command = `wmic process call create "\\"${execPath}\\" \\"${script}\\" --daemon"`; |
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.
style: Consider migrating from WMIC (deprecated in Windows 11+) to PowerShell Start-Process
| const command = `wmic process call create "\\"${execPath}\\" \\"${script}\\" --daemon"`; | |
| const command = `powershell -NoProfile -NonInteractive -WindowStyle Hidden -Command "Start-Process -FilePath '${execPath}' -ArgumentList '\\"${script}\\"','--daemon' -WindowStyle Hidden"`; |
The current WMIC approach works well, so this is a minor suggestion for future-proofing.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/services/infrastructure/ProcessManager.ts
Line: 294:294
Comment:
**style:** Consider migrating from WMIC (deprecated in Windows 11+) to PowerShell `Start-Process`
```suggestion
const command = `powershell -NoProfile -NonInteractive -WindowStyle Hidden -Command "Start-Process -FilePath '${execPath}' -ArgumentList '\\"${script}\\"','--daemon' -WindowStyle Hidden"`;
```
The current WMIC approach works well, so this is a minor suggestion for future-proofing.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Summary
cmd /c start /Bworkaround on Windows to allowwindowsHide: trueto workdetached: true+windowsHide: trueare incompatible on WindowsIssues
Fixes thedotmack#748, thedotmack#708, thedotmack#681, thedotmack#676
Test plan
🤖 Generated with Claude Code