fix(windows): resolve daemon project root in src mode; add start/stop…#171
fix(windows): resolve daemon project root in src mode; add start/stop…#171edenahoussou wants to merge 1 commit intonexu-io:mainfrom
Conversation
… scripts; approve sharp build
lefarcen
left a comment
lefarcen
left a comment
There was a problem hiding this comment.
Hey @edenahoussou, thanks for tackling this Windows dev-mode bug! 🎉 The daemon path fix is solid — treating both dist and src as daemon subdirs is exactly the right approach.
However, the PowerShell scripts introduce several issues that need addressing before merge. See inline comments below.
| @@ -0,0 +1,44 @@ | |||
| $ErrorActionPreference = "Stop" | |||
|
|
|||
| $repoRoot = "C:\Users\aeden\Documents\Codex\2026-04-29\https-github-com-nexu-io-open" | |||
There was a problem hiding this comment.
P1 Hardcoded absolute path breaks portability
This path is specific to your machine (C:\Users\aeden\Documents\Codex\2026-04-29\...). Anyone else who clones the repo will get a runtime error.
Fix: Use a dynamic resolution based on the script's location:
$repoRoot = Split-Path -Parent $PSScriptRootOr accept it as a script parameter:
param(
[string]$RepoRoot = (Split-Path -Parent $PSScriptRoot)
)| & $pnpmCmd tools-dev start daemon | Out-Host | ||
|
|
||
| $statusOutput = & $pnpmCmd tools-dev status | Out-String | ||
| $daemonUrl = [regex]::Match($statusOutput, 'http://127\.0\.0\.1:(\d+)').Value |
There was a problem hiding this comment.
P2 Regex extraction has no fallback validation
If the regex fails to match (e.g., tools-dev status output format changes), the script throws. But the error message won't tell the user what the actual output was.
Fix: Show the actual status output in the error:
if (-not $daemonUrl) {
Write-Error "Unable to detect daemon URL from tools-dev status. Output was:`n$statusOutput"
exit 1
}|
|
||
| $existing = Get-NetTCPConnection -LocalPort 3000 -State Listen -ErrorAction SilentlyContinue | ||
| if ($existing) { | ||
| $existing | ForEach-Object { |
There was a problem hiding this comment.
P2 Force-killing processes on port 3000 without confirmation
This will silently terminate ANY process listening on 3000, even if it's not a previous pnpm dev run (e.g., another dev server, Vite, etc.).
Fix: Add a confirmation prompt or at least log which process is being killed:
$existing | ForEach-Object {
$proc = Get-Process -Id $_.OwningProcess -ErrorAction SilentlyContinue
Write-Warning "Terminating process $($proc.Name) (PID $($proc.Id)) on port 3000"
Stop-Process -Id $_.OwningProcess -Force -ErrorAction SilentlyContinue
}| @@ -0,0 +1,19 @@ | |||
| $ErrorActionPreference = "SilentlyContinue" | |||
|
|
|||
| $repoRoot = "C:\Users\aeden\Documents\Codex\2026-04-29\https-github-com-nexu-io-open" | |||
There was a problem hiding this comment.
P1 Same hardcoded path issue as start script
See comment on start-open-design.ps1:3. Use $PSScriptRoot or script parameter.
| "electron", | ||
| "esbuild" | ||
| "esbuild", | ||
| "sharp" |
There was a problem hiding this comment.
P3 Why does sharp need explicit approval here?
The PR description mentions "environment/security-policy related" but doesn't explain which policy or why sharp triggers it.
Suggestion: Add a comment in the PR (or inline in package.json) explaining the context — e.g., "sharp v0.x.x triggers pnpm's native build confirmation due to postinstall compilation; approved after security audit on [date]".
This helps future maintainers understand why it's allowlisted.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e032ac04bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $repoRoot = "C:\Users\aeden\Documents\Codex\2026-04-29\https-github-com-nexu-io-open" | ||
| $webDir = Join-Path $repoRoot "apps\web" | ||
| $pnpmCmd = "C:\Program Files\nodejs\pnpm.cmd" |
There was a problem hiding this comment.
These scripts are hard-coded to one developer’s local paths (C:\Users\aeden\... and C:\Program Files\nodejs\pnpm.cmd), so they fail immediately on any other Windows machine before starting or stopping Open Design. Because they were added as general helper scripts, this makes them unusable for most contributors unless they manually edit the files first.
Useful? React with 👍 / 👎.
| $existing = Get-NetTCPConnection -LocalPort 3000 -State Listen -ErrorAction SilentlyContinue | ||
| if ($existing) { | ||
| Write-Host "Stopping web process on port 3000..." | ||
| $existing | ForEach-Object { | ||
| Stop-Process -Id $_.OwningProcess -Force -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
This unconditionally force-kills every process listening on port 3000, which can terminate unrelated local apps (e.g., another dev server) whenever this stop script runs. The bug appears whenever port 3000 is shared by non-Open Design processes, so shutdown should be scoped to the process started by this workflow rather than all listeners.
Useful? React with 👍 / 👎.
Lens B — Reasoning CompletenessBeyond the code issues above, a few design questions the PR doesn't address: 1. Cross-platform parity
2. CI/automation implications
3. Alternative approaches
Not saying those are better — just that the PR doesn't explain the trade-offs. Knowing why PowerShell was chosen (e.g., "need native Windows process control for reliable port cleanup") helps reviewers assess fit. 4. Prod vs dev boundary But what happens if someone accidentally runs from
These aren't blockers for the daemon fix itself (which is correct), but addressing them would make this PR production-ready for the scripting additions. Let me know if you'd like to iterate on the scripts or if you prefer to land the daemon fix separately and defer the Windows ergonomics to a follow-up PR. |
|
Hi @edenahoussou! 🎉 Thanks for the contribution — fixing the Windows dev-mode catalog empty issue is important for cross-platform dev experience. The daemon path resolution fix is solid. I've run a deep review (code correctness + design reasoning) and left inline comments on the PowerShell scripts. Once those are addressed, this will be ready to merge. Looking forward to seeing the updated version! — open-design team |
|
Hey @edenahoussou! 👋 I took another look at the PR with fresh eyes. The daemon path resolution fix ( handling both However, I noticed the PowerShell scripts have a few architecture concerns that would make them fragile for other contributors: 🚨 Hardcoded paths
Impact: These scripts will fail for anyone who doesn't have the exact same folder structure or pnpm install location. 💡 Suggested path forwardSince the daemon fix itself is the critical part and already solves the Windows catalog empty issue, here are two options: Option A: Make scripts portable (recommended if you'd like to keep them)Replace hardcoded paths with dynamic resolution: # Auto-detect repo root from script location
$repoRoot = Split-Path -Parent $PSScriptRoot
# Use pnpm from PATH (or detect via Get-Command)
$pnpmCmd = (Get-Command pnpm -ErrorAction Stop).SourceThis way the scripts work for any contributor on any Windows machine. Option B: Split the PR (faster path to merge)
Why this mattersopen-design already has No rush — take your time to decide which approach works best for you. The daemon fix is great work and we definitely want to land that. Let me know how you'd like to proceed! 🙌 |
|
Hi @edenahoussou! Just checking in — it's been about 38h since your last update, and there are still some review comments to address. No rush if you're busy. If anything in the feedback was unclear or you'd like a specific suggestion, just reply here. If you've moved on, feel free to close the PR. Thanks for the contribution! |
2 participants
Summary
This PR fixes a Windows dev-mode issue where Open Design launched correctly but loaded an empty catalog (no skills / design systems) in the UI, and adds helper scripts to make local startup more reliable.
Problem
On Windows, when running the daemon from source (apps/daemon/src), PROJECT_ROOT was resolved as if runtime always came from dist.
As a result, the daemon scanned the wrong folders for:
skills/
design-systems/
Observed behavior:
UI showed no templates/design systems
daemon API returned empty arrays:
/api/skills -> skills: []
/api/design-systems -> designSystems: []
Root Cause
resolveProjectRoot() only handled dist, not src:
dist path worked
src path resolved one level too high in dev mode
Changes
Daemon root resolution fix
File: apps/daemon/src/server.ts
Updated resolveProjectRoot() to treat both dist and src as daemon subdirs and normalize to the same repo root logic.
Startup ergonomics on Windows
Added start-open-design.ps1
uses Node 24.7.0
starts daemon
detects daemon URL/port
starts web on localhost:3000 with matching OD_PORT
Added stop-open-design.ps1
stops tools-dev services
kills lingering process bound to port 3000
Dependency build approval
package.json updated in pnpm.onlyBuiltDependencies to include sharp
reflects the explicit pnpm approve-builds step needed in this environment
Validation
Validated locally on Windows:
daemon running and reachable
API now returns populated catalogs:
skills_count=31
design_systems_count=72
web UI loads catalog correctly (templates/design systems visible)
CLI detection remains functional
startup scripts work end-to-end
Notes
This is a dev/runtime path resolution fix; no behavior change expected for production builds that run from dist.
The sharp approval entry is environment/security-policy related and keeps install flow deterministic on this setup.
How to test
pnpm install
pnpm tools-dev start daemon
or .\start-open-design.ps1
Then verify:
GET /api/skills returns non-empty skills
GET /api/design-systems returns non-empty designSystems
UI shows skills/design-system pickers populated.