added dev-setup and launch script for windows, linux and MacOS#1351
added dev-setup and launch script for windows, linux and MacOS#1351aliviahossain wants to merge 3 commits into
Conversation
|
|
|
Warning Review limit reached
Next review available in: 25 minutes Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable usage-based reviews in Billing to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information, and refer to the rate limits docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds ChangesDev Environment Bootstrap
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
SETUP.md (2)
27-28: 📐 Maintainability & Code Quality | 🔵 TrivialConsider
-Scope Processfor Set-ExecutionPolicy.
-Scope CurrentUserpersists the execution policy change. For a one-time setup script,-Scope Processis less invasive and sufficient for the current session.-Set-ExecutionPolicy -Scope CurrentUser -ExecutionPolicy RemoteSigned +Set-ExecutionPolicy -Scope Process -ExecutionPolicy RemoteSigned🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SETUP.md` around lines 27 - 28, The setup instructions use a persistent execution policy change, but this should be a one-time session-only adjustment. Update the PowerShell guidance to use the Set-ExecutionPolicy command with Process scope instead of CurrentUser, and keep the dev-setup.ps1 invocation unchanged so the policy applies only to the current session.
67-67: 📐 Maintainability & Code Quality | 🔵 TrivialUse American spelling for consistency.
"colour-coded" uses British spelling; the rest of the document and codebase likely use American English. Consider "color-coded" for consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SETUP.md` at line 67, The setup document uses British spelling in the log prefix description, which is inconsistent with the rest of the project. Update the wording in the relevant setup section to use the American spelling "color-coded" instead of "colour-coded" so the description stays consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev-setup.ps1`:
- Around line 1-275: Add automated Pester coverage for the dev bootstrap
contract in dev-setup.ps1, since the script currently relies on manual Windows
validation. Create tests around the main flow and helper behavior in
dev-setup.ps1 to verify repo-root validation, install-skip idempotency for
existing Node.js/Cargo/Conda dependencies, health-check handling after the Phase
4 Invoke-WebRequest calls, and cleanup in the final try/finally job shutdown.
Use the identifiable script helpers and flow points like Fail, Skip, Header, the
backend/sync/frontend job startup blocks, and the final Stop-Job/Remove-Job
cleanup to keep the tests resilient to refactors.
- Around line 231-249: The health-check flow in the dev setup script only warns
in the backend and sync microservice catch blocks, so the script still reports
success even when a service is unhealthy. Update the logic around the two
Invoke-WebRequest checks so that a failed check causes the run to fail instead
of continuing to the final success banner, using the existing Ok/Warn flow and
the backend/sync service checks as the locating symbols. Make sure the script
exits or throws after a failed health check so the “PictoPy dev environment is
up!” message is only printed when both services respond successfully.
- Around line 10-17: The native install steps in dev-setup.ps1 can succeed in
the script flow even when winget, npm, or conda fail, because
$ErrorActionPreference alone does not catch non-zero exit codes. Update the
install/check helpers and the affected setup steps that invoke native commands
so they inspect $LASTEXITCODE immediately after each call and route failures
through Fail before any Ok/continue logic. Focus on the native-command sections
in the setup flow and the helper functions like Ok, Skip, Warn, and Fail so the
script stops on real install errors instead of reporting success.
- Around line 259-260: The log-tag checks in the output coloring logic are using
pattern matching that does not match the emitted tags, so the `[BACKEND]` and
`[SYNC]` branches never fire. Update the conditional block that uses `$line` and
`Write-Host` to perform literal prefix checks with `StartsWith('[BACKEND]')` and
`StartsWith('[SYNC]')` instead of the current `-like` patterns, keeping the
existing color assignments unchanged.
In `@dev-setup.sh`:
- Around line 1-290: The dev-setup.sh bootstrap flow lacks automated smoke
coverage for its critical paths. Add non-interactive tests around the script’s
key branches, including detect_os, the “already installed” rerun paths,
dependency-install failure handling, and the Phase 4 health-check
success/failure branches. Use the script’s existing helpers and symbols like
detect_os, cleanup, and the backend/sync launch and curl health-check logic to
structure tests that verify the cross-platform setup and orchestration behavior
without manual intervention.
- Around line 187-193: The backend dependency setup in the install flow is using
a weak fastapi import check that can skip needed packages after interrupted
installs or requirements changes. Update the backend setup logic around the
dependency install/launch path to stop relying on the fastapi-only guard and
instead run the pip install step every time, or replace the guard with a real
requirements lock/hash check. Use the backend setup and launch symbols in
dev-setup.sh (the backend install block and the later launch check) to keep both
paths consistent.
- Around line 224-229: The shutdown logic in cleanup() only kills the stored
wrapper PIDs, which can leave the actual fastapi, npm, or Tauri child processes
running. Update the service launch path so each background service is started in
its own process group, then change cleanup() to terminate the entire group
instead of only the wrapper PID. Use the existing PIDS tracking and the service
startup blocks around the backend/sync/Tauri launches to keep the process-group
handling consistent.
- Line 75: The installer steps in dev-setup.sh run remote scripts directly,
including the NodeSource setup in the setup_20.x curl pipeline and the Miniconda
download/execution flow. Change both paths to download into a secure temporary
file created with mktemp, verify the vendor checksum or signature before
executing, and only then run the installer; update the Miniconda install block
and the NodeSource setup logic accordingly.
- Around line 267-286: The startup checks in dev-setup.sh currently only warn
when the backend or sync microservice is unreachable, but the script still
prints the success banner afterward. Update the health-check flow around the
backend and sync probes so that a failed curl check marks the startup as failed
and exits non-zero before reaching the final “PictoPy dev environment is up”
message. Use the existing ok/warn logic and the backend/sync check blocks as the
main points to locate and adjust the failure handling.
In `@SETUP.md`:
- Around line 159-162: The port cleanup command is unsafe when no process is
listening, because xargs still invokes kill with empty input. Update the setup
instructions’ port-kill example to use a guard or equivalent safe form that only
runs kill when lsof returns PIDs, and keep it cross-platform-friendly in the
setup guidance.
- Around line 140-144: The backend startup instructions are inconsistent with
the automated script and still reference running main.py manually. Update the
setup guidance that mentions the backend startup command to match the same
FastAPI dev invocation used elsewhere, and keep the command consistent with the
existing backend startup flow so users follow one correct path.
- Around line 88-98: The backend setup instructions are inconsistent with the
automated script because the manual command in the Backend section uses main.py
while the actual startup flow uses fastapi dev --port 52123. Update the Backend
steps to match the real entrypoint used by the project, or verify main.py is the
correct launcher and make both references consistent; use the Backend section
and the startup command as the symbols to align.
---
Nitpick comments:
In `@SETUP.md`:
- Around line 27-28: The setup instructions use a persistent execution policy
change, but this should be a one-time session-only adjustment. Update the
PowerShell guidance to use the Set-ExecutionPolicy command with Process scope
instead of CurrentUser, and keep the dev-setup.ps1 invocation unchanged so the
policy applies only to the current session.
- Line 67: The setup document uses British spelling in the log prefix
description, which is inconsistent with the rest of the project. Update the
wording in the relevant setup section to use the American spelling "color-coded"
instead of "colour-coded" so the description stays consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0dd8eab-a515-43b6-bf6b-a9e61e66ac59
📒 Files selected for processing (3)
SETUP.mddev-setup.ps1dev-setup.sh
| #!/bin/bash | ||
|
|
||
| # ============================================================================= | ||
| # PictoPy Dev Setup & Launch Script | ||
| # Supports: Linux (Debian/Ubuntu/Arch) and macOS | ||
| # Usage: bash dev-setup.sh | ||
| # ============================================================================= | ||
|
|
||
| set -e # exit on error | ||
|
|
||
| # ── Colors ─────────────────────────────────────────────────────────────────── | ||
| RED='\033[0;31m' | ||
| GREEN='\033[0;32m' | ||
| YELLOW='\033[1;33m' | ||
| BLUE='\033[0;34m' | ||
| CYAN='\033[0;36m' | ||
| BOLD='\033[1m' | ||
| NC='\033[0m' # No Color | ||
|
|
||
| # ── Helpers ─────────────────────────────────────────────────────────────────── | ||
| ok() { echo -e "${GREEN}✓${NC} $1"; } | ||
| skip() { echo -e "${CYAN}→${NC} $1 (already installed, skipping)"; } | ||
| info() { echo -e "${BLUE}▸${NC} $1"; } | ||
| warn() { echo -e "${YELLOW}⚠${NC} $1"; } | ||
| fail() { echo -e "${RED}✗${NC} $1"; exit 1; } | ||
| header() { | ||
| echo "" | ||
| echo -e "${BOLD}${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" | ||
| echo -e "${BOLD} $1${NC}" | ||
| echo -e "${BOLD}${BLUE}━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━${NC}" | ||
| } | ||
|
|
||
| # ── Detect OS ───────────────────────────────────────────────────────────────── | ||
| detect_os() { | ||
| if [[ "$OSTYPE" == "darwin"* ]]; then | ||
| OS="mac" | ||
| elif [[ -f /etc/debian_version ]]; then | ||
| OS="debian" | ||
| elif [[ -f /etc/arch-release ]]; then | ||
| OS="arch" | ||
| elif [[ -f /etc/fedora-release ]]; then | ||
| OS="fedora" | ||
| else | ||
| OS="linux-other" | ||
| fi | ||
| info "Detected OS: $OS" | ||
| } | ||
|
|
||
| # ── Get script directory (root of PictoPy) ──────────────────────────────────── | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| FRONTEND_DIR="$SCRIPT_DIR/frontend" | ||
| BACKEND_DIR="$SCRIPT_DIR/backend" | ||
| SYNC_DIR="$SCRIPT_DIR/sync-microservice" | ||
|
|
||
| # ── Validate we are in the PictoPy root ─────────────────────────────────────── | ||
| if [[ ! -d "$FRONTEND_DIR" || ! -d "$BACKEND_DIR" || ! -d "$SYNC_DIR" ]]; then | ||
| fail "Please run this script from the root of the PictoPy repository." | ||
| fi | ||
|
|
||
| # ============================================================================= | ||
| # PHASE 1 — PREREQUISITES | ||
| # ============================================================================= | ||
| header "Phase 1: Checking Prerequisites" | ||
|
|
||
| detect_os | ||
|
|
||
| # ── Node.js ─────────────────────────────────────────────────────────────────── | ||
| if command -v node &>/dev/null; then | ||
| skip "Node.js ($(node --version))" | ||
| else | ||
| info "Installing Node.js..." | ||
| if [[ "$OS" == "mac" ]]; then | ||
| brew install node || fail "Homebrew not found. Install it from https://brew.sh first." | ||
| elif [[ "$OS" == "debian" ]]; then | ||
| curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash - | ||
| sudo apt-get install -y nodejs | ||
| elif [[ "$OS" == "arch" ]]; then | ||
| sudo pacman -S --noconfirm nodejs npm | ||
| else | ||
| warn "Cannot auto-install Node.js on this OS. Please install it manually from https://nodejs.org" | ||
| fail "Node.js is required." | ||
| fi | ||
| ok "Node.js installed ($(node --version))" | ||
| fi | ||
|
|
||
| # ── Rust / Cargo ────────────────────────────────────────────────────────────── | ||
| if command -v cargo &>/dev/null; then | ||
| skip "Rust/Cargo ($(cargo --version))" | ||
| else | ||
| info "Installing Rust via rustup..." | ||
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | ||
| source "$HOME/.cargo/env" | ||
| ok "Rust installed ($(cargo --version))" | ||
| fi | ||
|
|
||
| # ── Tauri system dependencies (Linux only) ──────────────────────────────────── | ||
| if [[ "$OS" == "debian" ]]; then | ||
| info "Checking Tauri system dependencies..." | ||
| TAURI_DEPS=( | ||
| libwebkit2gtk-4.1-dev | ||
| libappindicator3-dev | ||
| librsvg2-dev | ||
| patchelf | ||
| libglib2.0-dev | ||
| libgl1-mesa-glx | ||
| pkg-config | ||
| build-essential | ||
| curl | ||
| wget | ||
| file | ||
| libxdo-dev | ||
| libssl-dev | ||
| libayatana-appindicator3-dev | ||
| ) | ||
| MISSING_DEPS=() | ||
| for dep in "${TAURI_DEPS[@]}"; do | ||
| dpkg -s "$dep" &>/dev/null || MISSING_DEPS+=("$dep") | ||
| done | ||
|
|
||
| if [[ ${#MISSING_DEPS[@]} -eq 0 ]]; then | ||
| skip "All Tauri system dependencies" | ||
| else | ||
| info "Installing missing system deps: ${MISSING_DEPS[*]}" | ||
| sudo apt-get update -qq | ||
| sudo apt-get install -y "${MISSING_DEPS[@]}" | ||
| ok "Tauri system dependencies installed" | ||
| fi | ||
| elif [[ "$OS" == "arch" ]]; then | ||
| info "Checking Tauri system dependencies (Arch)..." | ||
| sudo pacman -S --needed --noconfirm webkit2gtk base-devel curl wget file openssl appmenu-gtk-module gtk3 libappindicator-gtk3 librsvg libvips | ||
| ok "Tauri system dependencies installed" | ||
| elif [[ "$OS" == "mac" ]]; then | ||
| skip "Tauri system dependencies (not needed on macOS)" | ||
| fi | ||
|
|
||
| # ── Miniconda ───────────────────────────────────────────────────────────────── | ||
| if command -v conda &>/dev/null; then | ||
| skip "Miniconda/Conda ($(conda --version))" | ||
| else | ||
| info "Installing Miniconda..." | ||
| if [[ "$OS" == "mac" ]]; then | ||
| MINICONDA_URL="https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-arm64.sh" | ||
| # Use x86_64 if not Apple Silicon | ||
| [[ "$(uname -m)" != "arm64" ]] && MINICONDA_URL="https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-x86_64.sh" | ||
| else | ||
| MINICONDA_URL="https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-x86_64.sh" | ||
| fi | ||
| curl -fsSL "$MINICONDA_URL" -o /tmp/miniconda.sh | ||
| bash /tmp/miniconda.sh -b -p "$HOME/miniconda3" | ||
| rm /tmp/miniconda.sh | ||
| export PATH="$HOME/miniconda3/bin:$PATH" | ||
| conda init bash 2>/dev/null || true | ||
| ok "Miniconda installed" | ||
| warn "You may need to restart your terminal after setup for conda to work globally." | ||
| fi | ||
|
|
||
| # Make sure conda is available in this shell session | ||
| CONDA_BIN=$(conda info --base 2>/dev/null)/bin/conda | ||
| export PATH="$(conda info --base 2>/dev/null)/bin:$PATH" | ||
|
|
||
| # ============================================================================= | ||
| # PHASE 2 — SERVICE SETUP | ||
| # ============================================================================= | ||
| header "Phase 2: Setting Up Services" | ||
|
|
||
| # ── Frontend ────────────────────────────────────────────────────────────────── | ||
| info "Frontend: checking node_modules..." | ||
| if [[ -d "$FRONTEND_DIR/node_modules" ]]; then | ||
| skip "Frontend npm dependencies" | ||
| else | ||
| info "Frontend: running npm install..." | ||
| (cd "$FRONTEND_DIR" && npm install) | ||
| ok "Frontend dependencies installed" | ||
| fi | ||
|
|
||
| # ── Backend ─────────────────────────────────────────────────────────────────── | ||
| info "Backend: checking conda environment..." | ||
| BACKEND_ENV="$BACKEND_DIR/.env" | ||
| if [[ -d "$BACKEND_ENV" ]]; then | ||
| skip "Backend conda environment (.env)" | ||
| else | ||
| info "Backend: creating conda environment with Python 3.12..." | ||
| conda create -p "$BACKEND_ENV" python=3.12 -y | ||
| ok "Backend conda environment created" | ||
| fi | ||
|
|
||
| info "Backend: installing Python dependencies..." | ||
| # Check if deps are already installed by testing a key package | ||
| if conda run -p "$BACKEND_ENV" python -c "import fastapi" &>/dev/null; then | ||
| skip "Backend Python packages" | ||
| else | ||
| conda run -p "$BACKEND_ENV" pip install -r "$BACKEND_DIR/requirements.txt" | ||
| ok "Backend Python packages installed" | ||
| fi | ||
|
|
||
| # ── Sync Microservice ───────────────────────────────────────────────────────── | ||
| info "Sync microservice: checking conda environment..." | ||
| SYNC_ENV="$SYNC_DIR/.sync-env" | ||
| if [[ -d "$SYNC_ENV" ]]; then | ||
| skip "Sync microservice conda environment (.sync-env)" | ||
| else | ||
| info "Sync microservice: creating conda environment with Python 3.12..." | ||
| conda create -p "$SYNC_ENV" python=3.12 -y | ||
| ok "Sync microservice conda environment created" | ||
| fi | ||
|
|
||
| info "Sync microservice: installing Python dependencies..." | ||
| if conda run -p "$SYNC_ENV" python -c "import fastapi" &>/dev/null; then | ||
| skip "Sync microservice Python packages" | ||
| else | ||
| conda run -p "$SYNC_ENV" pip install -r "$SYNC_DIR/requirements.txt" | ||
| ok "Sync microservice Python packages installed" | ||
| fi | ||
|
|
||
| # ============================================================================= | ||
| # PHASE 3 — LAUNCH ALL SERVICES | ||
| # ============================================================================= | ||
| header "Phase 3: Launching All Services" | ||
|
|
||
| # Store background process PIDs for cleanup | ||
| PIDS=() | ||
|
|
||
| # Cleanup on Ctrl+C or exit | ||
| cleanup() { | ||
| echo "" | ||
| warn "Shutting down all services..." | ||
| for pid in "${PIDS[@]}"; do | ||
| kill "$pid" 2>/dev/null || true | ||
| done | ||
| ok "All services stopped. Goodbye!" | ||
| exit 0 | ||
| } | ||
| trap cleanup SIGINT SIGTERM | ||
|
|
||
| # ── Launch Backend ──────────────────────────────────────────────────────────── | ||
| info "Starting backend on port 52123..." | ||
| ( | ||
| cd "$BACKEND_DIR" | ||
| conda run -p "$BACKEND_ENV" fastapi dev --port 52123 2>&1 | \ | ||
| while IFS= read -r line; do echo -e "${GREEN}[BACKEND]${NC} $line"; done | ||
| ) & | ||
| PIDS+=($!) | ||
|
|
||
| # ── Launch Sync Microservice ────────────────────────────────────────────────── | ||
| info "Starting sync microservice on port 52124..." | ||
| ( | ||
| cd "$SYNC_DIR" | ||
| conda run -p "$SYNC_ENV" fastapi dev --port 52124 2>&1 | \ | ||
| while IFS= read -r line; do echo -e "${YELLOW}[SYNC]${NC} $line"; done | ||
| ) & | ||
| PIDS+=($!) | ||
|
|
||
| # ── Launch Frontend ─────────────────────────────────────────────────────────── | ||
| info "Starting Tauri frontend..." | ||
| ( | ||
| cd "$FRONTEND_DIR" | ||
| npm run tauri dev 2>&1 | \ | ||
| while IFS= read -r line; do echo -e "${BLUE}[FRONTEND]${NC} $line"; done | ||
| ) & | ||
| PIDS+=($!) | ||
|
|
||
| # ============================================================================= | ||
| # PHASE 4 — HEALTH CHECK | ||
| # ============================================================================= | ||
| header "Phase 4: Health Check" | ||
|
|
||
| info "Waiting 10 seconds for services to start..." | ||
| sleep 10 | ||
|
|
||
| # Check backend | ||
| if curl -s "http://localhost:52123" &>/dev/null || curl -s "http://localhost:52123/docs" &>/dev/null; then | ||
| ok "Backend is running at http://localhost:52123" | ||
| else | ||
| warn "Backend may still be starting up — check [BACKEND] logs above" | ||
| fi | ||
|
|
||
| # Check sync microservice | ||
| if curl -s "http://localhost:52124" &>/dev/null || curl -s "http://localhost:52124/docs" &>/dev/null; then | ||
| ok "Sync microservice is running at http://localhost:52124" | ||
| else | ||
| warn "Sync microservice may still be starting — check [SYNC] logs above" | ||
| fi | ||
|
|
||
| echo "" | ||
| echo -e "${BOLD}${GREEN}✓ PictoPy dev environment is up!${NC}" | ||
| echo -e " Press ${BOLD}Ctrl+C${NC} to stop all services." | ||
| echo "" | ||
|
|
||
| # Keep script alive so logs keep streaming and Ctrl+C works | ||
| wait No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
This bootstrap flow needs automated smoke coverage.
This PR adds cross-platform setup, idempotency, launch orchestration, and health-check logic, but no automated tests are shown for the critical paths. At minimum, add non-interactive smoke tests for OS detection, “already installed” reruns, dependency-install failure handling, and the health-check success/failure branches. As per path instructions, “Ensure that test code is automated, comprehensive, and follows testing best practices” and “Verify that all critical functionality is covered by tests.”
🧰 Tools
🪛 ast-grep (0.44.0)
[error] 74-74: Remote content fetched with curl/wget is piped directly into a shell interpreter, so any server compromise, MITM, or tampered mirror results in arbitrary code execution on this host. Download the script to a file first, verify its integrity (checksum/signature) and inspect it, then run the verified local copy.
Context: curl -fsSL https://deb.nodesource.com/setup_20.x | sudo -E bash -
Note: [CWE-494] Download of Code Without Integrity Check.
(curl-pipe-to-shell-bash)
[error] 90-90: Remote content fetched with curl/wget is piped directly into a shell interpreter, so any server compromise, MITM, or tampered mirror results in arbitrary code execution on this host. Download the script to a file first, verify its integrity (checksum/signature) and inspect it, then run the verified local copy.
Context: curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y
Note: [CWE-494] Download of Code Without Integrity Check.
(curl-pipe-to-shell-bash)
[warning] 147-147: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/miniconda.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 148-148: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/miniconda.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 149-149: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/miniconda.sh
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🪛 Shellcheck (0.11.0)
[info] 92-92: Not following: ./.cargo/env was not specified as input (see shellcheck -x).
(SC1091)
[warning] 158-158: CONDA_BIN appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 159-159: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev-setup.sh` around lines 1 - 290, The dev-setup.sh bootstrap flow lacks
automated smoke coverage for its critical paths. Add non-interactive tests
around the script’s key branches, including detect_os, the “already installed”
rerun paths, dependency-install failure handling, and the Phase 4 health-check
success/failure branches. Use the script’s existing helpers and symbols like
detect_os, cleanup, and the backend/sync launch and curl health-check logic to
structure tests that verify the cross-platform setup and orchestration behavior
without manual intervention.
Source: Path instructions
| info "Waiting 10 seconds for services to start..." | ||
| sleep 10 | ||
|
|
||
| # Check backend | ||
| if curl -s "http://localhost:52123" &>/dev/null || curl -s "http://localhost:52123/docs" &>/dev/null; then | ||
| ok "Backend is running at http://localhost:52123" | ||
| else | ||
| warn "Backend may still be starting up — check [BACKEND] logs above" | ||
| fi | ||
|
|
||
| # Check sync microservice | ||
| if curl -s "http://localhost:52124" &>/dev/null || curl -s "http://localhost:52124/docs" &>/dev/null; then | ||
| ok "Sync microservice is running at http://localhost:52124" | ||
| else | ||
| warn "Sync microservice may still be starting — check [SYNC] logs above" | ||
| fi | ||
|
|
||
| echo "" | ||
| echo -e "${BOLD}${GREEN}✓ PictoPy dev environment is up!${NC}" | ||
| echo -e " Press ${BOLD}Ctrl+C${NC} to stop all services." |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don't print a success banner after failed health checks.
If either probe fails, the script only warns and still prints ✓ PictoPy dev environment is up!. That contradicts the PR objective that Phase 4 verifies both Python servers after startup and makes broken launches look successful. Fail the script when either service is still unreachable after the wait/retry window.
Suggested direction
-header "Phase 4: Health Check"
+header "Phase 4: Health Check"
+health_failed=0
@@
-if curl -s "http://localhost:52123" &>/dev/null || curl -s "http://localhost:52123/docs" &>/dev/null; then
+if curl -fsS "http://localhost:52123/docs" &>/dev/null; then
ok "Backend is running at http://localhost:52123"
else
- warn "Backend may still be starting up — check [BACKEND] logs above"
+ fail "Backend failed health check — check [BACKEND] logs above"
fi
@@
-if curl -s "http://localhost:52124" &>/dev/null || curl -s "http://localhost:52124/docs" &>/dev/null; then
+if curl -fsS "http://localhost:52124/docs" &>/dev/null; then
ok "Sync microservice is running at http://localhost:52124"
else
- warn "Sync microservice may still be starting — check [SYNC] logs above"
+ fail "Sync microservice failed health check — check [SYNC] logs above"
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| info "Waiting 10 seconds for services to start..." | |
| sleep 10 | |
| # Check backend | |
| if curl -s "http://localhost:52123" &>/dev/null || curl -s "http://localhost:52123/docs" &>/dev/null; then | |
| ok "Backend is running at http://localhost:52123" | |
| else | |
| warn "Backend may still be starting up — check [BACKEND] logs above" | |
| fi | |
| # Check sync microservice | |
| if curl -s "http://localhost:52124" &>/dev/null || curl -s "http://localhost:52124/docs" &>/dev/null; then | |
| ok "Sync microservice is running at http://localhost:52124" | |
| else | |
| warn "Sync microservice may still be starting — check [SYNC] logs above" | |
| fi | |
| echo "" | |
| echo -e "${BOLD}${GREEN}✓ PictoPy dev environment is up!${NC}" | |
| echo -e " Press ${BOLD}Ctrl+C${NC} to stop all services." | |
| info "Waiting 10 seconds for services to start..." | |
| sleep 10 | |
| # Check backend | |
| if curl -fsS "http://localhost:52123/docs" &>/dev/null; then | |
| ok "Backend is running at http://localhost:52123" | |
| else | |
| fail "Backend failed health check — check [BACKEND] logs above" | |
| fi | |
| # Check sync microservice | |
| if curl -fsS "http://localhost:52124/docs" &>/dev/null; then | |
| ok "Sync microservice is running at http://localhost:52124" | |
| else | |
| fail "Sync microservice failed health check — check [SYNC] logs above" | |
| fi | |
| echo "" | |
| echo -e "${BOLD}${GREEN}✓ PictoPy dev environment is up!${NC}" | |
| echo -e " Press ${BOLD}Ctrl+C${NC} to stop all services." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev-setup.sh` around lines 267 - 286, The startup checks in dev-setup.sh
currently only warn when the backend or sync microservice is unreachable, but
the script still prints the success banner afterward. Update the health-check
flow around the backend and sync probes so that a failed curl check marks the
startup as failed and exits non-zero before reaching the final “PictoPy dev
environment is up” message. Use the existing ok/warn logic and the backend/sync
check blocks as the main points to locate and adjust the failure handling.
|
|
1 similar comment
|
|
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dev-setup.ps1 (2)
192-224: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winStart the cleanup scope before launching background jobs.
The
finallyonly protects the log-streaming loop. If the user presses Ctrl+C during the 10-second wait or health-check phase, the backend/sync/frontend jobs can be left running. Move thetryto wrap Phase 3 and Phase 4 from before the firstStart-Job.Suggested structure
$jobs = @() +try { # ── Launch Backend ──────────────────────────────────────────────────────────── Info "Starting backend on port 52123..." ... -try { while ($true) { foreach ($job in $jobs) { ... } Start-Sleep -Milliseconds 500 } } finally {Also applies to: 255-275
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev-setup.ps1` around lines 192 - 224, The cleanup scope currently starts too late, so Ctrl+C during the startup wait or health-check can leave the jobs from Start-Job running. Move the try/finally in dev-setup.ps1 so it wraps Phase 3 and Phase 4 from before the first Start-Job through the health-check section, ensuring the cleanup logic still sees $jobs and can stop the backend, sync, and frontend jobs if interruption happens early.
130-138: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse manifest-aware dependency checks here and in the backend/sync blocks.
node_modulesandimport fastapionly show that something is present, not that it matchespackage-lock.jsonor eitherrequirements.txt. If those manifests change, these branches can still skip installs and leave stale dependencies behind.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev-setup.ps1` around lines 130 - 138, The dependency skip checks in the frontend install path, and the matching backend/sync install blocks, only test for presence of node_modules or import fastapi instead of validating against the manifests. Update the logic in dev-setup.ps1 to use manifest-aware checks tied to package-lock.json and the relevant requirements.txt files so installs rerun whenever those manifests change. Keep the existing flow in the frontend block and the backend/sync sections, but base the “Skip” decision on manifest freshness rather than directory/module presence.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev-setup.ps1`:
- Around line 13-22: The helper status functions in dev-setup.ps1 use non-ASCII
symbols that can be misread by PowerShell 5.1 when the file lacks a BOM. Update
the script so the messages in Ok, Skip, Info, Warn, Fail, and Header are either
saved as UTF-8 with BOM or changed to plain ASCII text, keeping the same helper
function names and behavior.
---
Outside diff comments:
In `@dev-setup.ps1`:
- Around line 192-224: The cleanup scope currently starts too late, so Ctrl+C
during the startup wait or health-check can leave the jobs from Start-Job
running. Move the try/finally in dev-setup.ps1 so it wraps Phase 3 and Phase 4
from before the first Start-Job through the health-check section, ensuring the
cleanup logic still sees $jobs and can stop the backend, sync, and frontend jobs
if interruption happens early.
- Around line 130-138: The dependency skip checks in the frontend install path,
and the matching backend/sync install blocks, only test for presence of
node_modules or import fastapi instead of validating against the manifests.
Update the logic in dev-setup.ps1 to use manifest-aware checks tied to
package-lock.json and the relevant requirements.txt files so installs rerun
whenever those manifests change. Keep the existing flow in the frontend block
and the backend/sync sections, but base the “Skip” decision on manifest
freshness rather than directory/module presence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c2c0a4b-8851-4d7d-adaf-57687b262463
📒 Files selected for processing (1)
dev-setup.ps1
| function Ok($msg) { Write-Host "✓ $msg" -ForegroundColor Green } | ||
| function Skip($msg) { Write-Host "→ $msg (already installed, skipping)" -ForegroundColor Cyan } | ||
| function Info($msg) { Write-Host "▸ $msg" -ForegroundColor Blue } | ||
| function Warn($msg) { Write-Host "⚠ $msg" -ForegroundColor Yellow } | ||
| function Fail($msg) { Write-Host "✗ $msg" -ForegroundColor Red; exit 1 } | ||
| function Header($msg) { | ||
| Write-Host "" | ||
| Write-Host "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" -ForegroundColor Blue | ||
| Write-Host " $msg" -ForegroundColor White | ||
| Write-Host "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" -ForegroundColor Blue |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
p = Path("dev-setup.ps1")
b = p.read_bytes()
print("has_utf8_bom=", b.startswith(b"\xef\xbb\xbf"))
print("contains_non_ascii=", any(byte >= 128 for byte in b))
PYRepository: AOSSIE-Org/PictoPy
Length of output: 201
Save dev-setup.ps1 as UTF-8 with BOM or switch these status strings to ASCII. PowerShell 5.1 can misread UTF-8 files without a BOM when they contain non-ASCII glyphs, which will garble these helper messages. dev-setup.ps1:13-22
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'dev-setup.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev-setup.ps1` around lines 13 - 22, The helper status functions in
dev-setup.ps1 use non-ASCII symbols that can be misread by PowerShell 5.1 when
the file lacks a BOM. Update the script so the messages in Ok, Skip, Info, Warn,
Fail, and Header are either saved as UTF-8 with BOM or changed to plain ASCII
text, keeping the same helper function names and behavior.
Source: Linters/SAST tools
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@aliviahossain, could you clarify what issue this PR is intended to resolve? The repository already contains a setup script, so it would have been better to update that file instead of adding new scripts. Also, adding these scripts directly to the root directory isn't the right approach. Going forward, We'd all appreciate it if you could open an issue before starting work on a PR. If you're unsure whether something needs to be addressed, feel free to ping me on Discord first so we can discuss it before you invest time into the implementation. For this PR, could you also share a short video demonstrating the implementation? I'll review the functionality based on that and determine whether the changes are valid. |
2 participants
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist
Summary
Setting up PictoPy for development currently requires manually running 3 separate terminals, installing multiple prerequisites in the right order, and knowing which conda env belongs to which service. Many contributors give up during this step.
This PR adds two scripts that automate the entire process — from a fresh machine to all 3 services running — with a single command.
Scripts Added
dev-setup.sh— Linux and macOS (Bash)dev-setup.ps1— Windows (PowerShell)What Each Script Does
Phase 1 — Prerequisites: Checks for Node.js, Rust/Cargo, Miniconda, and platform-specific Tauri dependencies (webkit2gtk on Linux, VS C++ Build Tools + WebView2 on Windows). Installs anything missing, skips anything already present.
Phase 2 — Service Setup: Creates conda environments for backend and sync-microservice if they don't exist, installs pip dependencies if not already installed, and runs npm install for the frontend if node_modules is absent. All steps are idempotent.
Phase 3 — Launch: Starts all 3 services (backend :52123, sync-microservice :52124, Tauri frontend) in a single terminal with
color-coded log prefixes [BACKEND], [SYNC], [FRONTEND]. Ctrl+C stops all 3 cleanly.
Phase 4 — Health Check: Pings both Python servers after 10 seconds to confirm they came up successfully.
On Re-runs
Every check is idempotent. Running the script a second time on an already-configured machine skips all installation steps and goes straight to launching services.
Tested On
Summary by CodeRabbit
Documentation
New Features