Skip to content

fix(windows): resolve daemon project root in src mode; add start/stop… #171

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.

Sign up for GitHub

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

Open
edenahoussou wants to merge 1 commit into
base: main
Choose a base branch
from
+67 −2
Open

fix(windows): resolve daemon project root in src mode; add start/stop… #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/daemon/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ import {
const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
export function resolveProjectRoot(moduleDir: string): string {
const daemonDir = path.basename(moduleDir) === 'dist'
const base = path.basename(moduleDir);
const daemonDir = base === 'dist' || base === 'src'
? path.dirname(moduleDir)
: moduleDir;
return path.resolve(daemonDir, '../..');
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@
"onlyBuiltDependencies": [
"better-sqlite3",
"electron",
"esbuild"
"esbuild",
"sharp"
Copy link
Copy Markdown
Contributor

@lefarcen lefarcen Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

]
}
}
44 changes: 44 additions & 0 deletions start-open-design.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
$ErrorActionPreference = "Stop"

$repoRoot = "C:\Users\aeden\Documents\Codex\2026-04-29\https-github-com-nexu-io-open"
Copy link
Copy Markdown
Contributor

@lefarcen lefarcen Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 $PSScriptRoot

Or accept it as a script parameter:

param(
  [string]$RepoRoot = (Split-Path -Parent $PSScriptRoot)
)

$webDir = Join-Path $repoRoot "apps\web"
$pnpmCmd = "C:\Program Files\nodejs\pnpm.cmd"
Comment on lines +3 to +5
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Derive repo/pnpm paths dynamically in startup scripts

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 👍 / 👎.

$nodeVersion = "24.7.0"

Set-Location $repoRoot

Write-Host "Using Node $nodeVersion..."
nvm use $nodeVersion | Out-Host

Write-Host "Starting daemon..."
& $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
Copy link
Copy Markdown
Contributor

@lefarcen lefarcen Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}

if (-not $daemonUrl) {
throw "Unable to detect daemon URL from tools-dev status."
}
$daemonPort = [regex]::Match($daemonUrl, ':(\d+)$').Groups[1].Value

Write-Host "Daemon URL: $daemonUrl"
Write-Host "Restarting web dev server on localhost:3000..."

$existing = Get-NetTCPConnection -LocalPort 3000 -State Listen -ErrorAction SilentlyContinue
if ($existing) {
$existing | ForEach-Object {
Copy link
Copy Markdown
Contributor

@lefarcen lefarcen Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
}

Stop-Process -Id $_.OwningProcess -Force -ErrorAction SilentlyContinue
}
}

$env:OD_PORT = $daemonPort
Start-Process -FilePath $pnpmCmd -ArgumentList "dev" -WorkingDirectory $webDir

Start-Sleep -Seconds 6
$listening = Get-NetTCPConnection -LocalPort 3000 -State Listen -ErrorAction SilentlyContinue
if (-not $listening) {
Write-Warning "Web server not detected on port 3000 yet. Check terminal logs in apps/web."
} else {
Write-Host "Open Design is running:"
Write-Host "- Daemon: $daemonUrl"
Write-Host "- Web: http://localhost:3000"
}
19 changes: 19 additions & 0 deletions stop-open-design.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
$ErrorActionPreference = "SilentlyContinue"

$repoRoot = "C:\Users\aeden\Documents\Codex\2026-04-29\https-github-com-nexu-io-open"
Copy link
Copy Markdown
Contributor

@lefarcen lefarcen Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Same hardcoded path issue as start script

See comment on start-open-design.ps1:3. Use $PSScriptRoot or script parameter.

$pnpmCmd = "C:\Program Files\nodejs\pnpm.cmd"

Set-Location $repoRoot

Write-Host "Stopping tools-dev services..."
& $pnpmCmd tools-dev stop | Out-Host

$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
Comment on lines +11 to +15
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid killing unrelated services bound to port 3000

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 👍 / 👎.

}
}

Write-Host "Open Design stopped."