Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 31 additions & 4 deletions studio/setup.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -748,19 +748,46 @@ Write-Host ""
# ==========================================================================
# PHASE 2: Frontend build (skip if pip-installed -- already bundled)
# ==========================================================================
$DistDir = Join-Path $FrontendDir "dist"
# Skip build if dist/ exists and no tracked input is newer than dist/.
# Checks src/, public/, package.json, config files -- not just src/.
$NeedFrontendBuild = $true
if ($IsPipInstall) {
$NeedFrontendBuild = $false
Write-Host "[OK] Running from pip install - frontend already bundled, skipping build" -ForegroundColor Green
} else {
} elseif (Test-Path $DistDir) {
$DistTime = (Get-Item $DistDir).LastWriteTime
# Check src/ and public/ recursively using explicit paths
$TrackedDirs = @("src", "public") |
ForEach-Object { Join-Path $FrontendDir $_ } |
Where-Object { Test-Path $_ }
$NewerFile = $null
if ($TrackedDirs.Count -gt 0) {
$NewerFile = $TrackedDirs |
ForEach-Object { Get-ChildItem -Path $_ -Recurse -File -ErrorAction SilentlyContinue } |
Where-Object { $_.LastWriteTime -gt $DistTime } | Select-Object -First 1
}
# Also check top-level config and entry files (package.json, vite.config.ts, index.html, etc.)
if (-not $NewerFile) {
$NewerFile = Get-ChildItem -Path $FrontendDir -File -ErrorAction SilentlyContinue |
Where-Object { $_.Name -match '\.(json|ts|js|mjs|html)$' -and $_.LastWriteTime -gt $DistTime } |
Select-Object -First 1
}
if (-not $NewerFile) {
$NeedFrontendBuild = $false
Write-Host "[OK] Frontend already built and up to date -- skipping build" -ForegroundColor Green
} else {
Write-Host "[INFO] Frontend source changed since last build -- rebuilding..." -ForegroundColor Yellow
}
}
if ($NeedFrontendBuild -and -not $IsPipInstall) {
Write-Host ""
Write-Host "Building frontend..." -ForegroundColor Cyan
# npm writes warnings to stderr; lower ErrorActionPreference so PS doesn't
# treat them as terminating errors (same pattern as the pip section below).
$prevEAP_npm = $ErrorActionPreference
$ErrorActionPreference = "Continue"
Push-Location $FrontendDir
# Remove stale node_modules and package-lock.json to avoid version conflicts
if (Test-Path "node_modules") { Remove-Item -Recurse -Force "node_modules" }
if (Test-Path "package-lock.json") { Remove-Item -Force "package-lock.json" }
npm install 2>&1 | Out-Null
if ($LASTEXITCODE -ne 0) {
Pop-Location
Expand Down
39 changes: 29 additions & 10 deletions studio/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,27 @@ if [[ "$keynames" == *$'\nCOLAB_'* ]]; then
fi

# ── Detect whether frontend needs building ──
# Only skip when BOTH conditions are true:
# 1. We're inside site-packages (PyPI / pip install, not editable)
# 2. dist/ already exists (pre-built in the wheel)
# Otherwise always (re)build — handles upgrades, editable installs, and
# pip-from-source where dist/ was never built.
if [[ "$SCRIPT_DIR" == */site-packages/* ]] && [ -d "$SCRIPT_DIR/frontend/dist" ]; then
echo "✅ Frontend pre-built (PyPI) — skipping Node/npm check."
# Skip if dist/ exists AND no tracked input is newer than dist/.
# Checks top-level config/entry files and src/, public/ recursively.
# This handles: PyPI installs (dist/ bundled), repeat runs (no changes),
# and upgrades/pulls (source newer than dist/ triggers rebuild).
_NEED_FRONTEND_BUILD=true
if [ -d "$SCRIPT_DIR/frontend/dist" ]; then
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Rebuild frontend when source code changes

This new guard skips the entire frontend build whenever frontend/dist already exists, so a developer who runs setup.sh, pulls new frontend changes, and runs setup again will keep serving stale assets from the previous commit. That can produce UI/backend contract mismatches (new backend endpoints or payloads with old JS bundle) even though setup reports success; the previous logic rebuilt for editable/source installs specifically to avoid this upgrade path regression.

Useful? React with 👍 / 👎.

# Check top-level config and entry files (package.json, vite.config.ts, index.html, etc.)
_changed=$(find "$SCRIPT_DIR/frontend" -maxdepth 1 \
\( -name "*.json" -o -name "*.ts" -o -name "*.js" -o -name "*.mjs" -o -name "*.html" \) \
-newer "$SCRIPT_DIR/frontend/dist" -print -quit 2>/dev/null)
Comment on lines +51 to +53
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The find command here is missing the -type f predicate. Without it, the command could potentially match a directory with a name like config.js. While unlikely, adding -type f would make the check more precise by ensuring only files are considered, which aligns with the intent of checking for changes in configuration files.

Suggested change
_changed=$(find "$SCRIPT_DIR/frontend" -maxdepth 1 \
\( -name "*.json" -o -name "*.ts" -o -name "*.js" -o -name "*.mjs" -o -name "*.html" \) \
-newer "$SCRIPT_DIR/frontend/dist" -print -quit 2>/dev/null)
_changed=$(find "$SCRIPT_DIR/frontend" -maxdepth 1 -type f \
\( -name "*.json" -o -name "*.ts" -o -name "*.js" -o -name "*.mjs" -o -name "*.html" \) \
-newer "$SCRIPT_DIR/frontend/dist" -print -quit 2>/dev/null)

# Check src/ and public/ recursively
if [ -z "$_changed" ]; then
_changed=$(find "$SCRIPT_DIR/frontend/src" "$SCRIPT_DIR/frontend/public" \
-type f -newer "$SCRIPT_DIR/frontend/dist" -print -quit 2>/dev/null)
Comment on lines +56 to +57
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard recursive find against missing frontend dirs

With set -euo pipefail, the find "$SCRIPT_DIR/frontend/src" "$SCRIPT_DIR/frontend/public" ... command substitution exits the whole setup when either directory is absent and no earlier match triggers -quit. That makes cached runs fail in installs that omit one of these folders (for example, slimmed package layouts), even though the intended behavior is to treat “no tracked changes” as a cache hit. Build freshness probing should only pass existing paths to find (or otherwise neutralize missing-path exit codes).

Useful? React with 👍 / 👎.

fi
if [ -z "$_changed" ]; then
_NEED_FRONTEND_BUILD=false
fi
fi
if [ "$_NEED_FRONTEND_BUILD" = false ]; then
echo "✅ Frontend already built and up to date -- skipping Node/npm check."
Comment on lines +63 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Decouple oxc-validator install from frontend cache gate

This condition now controls the whole Node block, including the later run_quiet "npm install (oxc validator runtime)" call in the same else block. On reruns where dist is considered up to date, setup skips reinstalling backend/core/data_recipe/oxc-validator dependencies, so dependency updates in that package are not applied after a pull.

Useful? React with 👍 / 👎.

else
NEED_NODE=true
if command -v node &>/dev/null && command -v npm &>/dev/null; then
Expand Down Expand Up @@ -146,12 +160,17 @@ run_quiet "npm run build" npm run build

_restore_gitignores
trap - EXIT
cd "$SCRIPT_DIR/backend/core/data_recipe/oxc-validator"
run_quiet "npm install (oxc validator runtime)" npm install
cd "$SCRIPT_DIR"
echo "✅ Frontend built to frontend/dist"

fi # end frontend dist check
fi # end frontend build check

# ── oxc-validator runtime (always install, independent of frontend build) ──
if [ -d "$SCRIPT_DIR/backend/core/data_recipe/oxc-validator" ] && command -v npm &>/dev/null; then
cd "$SCRIPT_DIR/backend/core/data_recipe/oxc-validator"
run_quiet "npm install (oxc validator runtime)" npm install
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Install Node before unconditional oxc-validator npm step

This npm install now runs even when _NEED_FRONTEND_BUILD=false, but the Node/npm bootstrap is only executed inside the frontend-build path. On environments that have prebuilt frontend/dist but no Node (a common pip/wheel setup path), setup now reaches this command and exits with 127 (npm: command not found) instead of provisioning prerequisites.

Useful? React with 👍 / 👎.

cd "$SCRIPT_DIR"
fi

# ── 6. Python venv + deps ──

Expand Down
Loading