Skip to content

feat: add one-command environment setup script#809

Closed
LaphoqueRC wants to merge 8 commits intoSolFoundry:mainfrom
LaphoqueRC:feat/bounty-491-env-setup-script
Closed

feat: add one-command environment setup script#809
LaphoqueRC wants to merge 8 commits intoSolFoundry:mainfrom
LaphoqueRC:feat/bounty-491-env-setup-script

Conversation

@LaphoqueRC
Copy link
Contributor

Closes #491

Adds scripts/dev-setup.sh for one-command development setup.

Usage

./scripts/dev-setup.sh              # Full setup with Docker
./scripts/dev-setup.sh --no-docker  # Manual setup
./scripts/dev-setup.sh --check      # Verify prerequisites

Steps

  1. Checks prerequisites (git, node 18+, python 3.10+, docker)
  2. Creates .env from .env.example
  3. Installs frontend deps (npm install)
  4. Creates Python venv + installs backend deps
  5. Starts Docker Compose services
  6. Health check with retries
  7. Prints service URLs

Features

  • Color-coded output
  • Version validation
  • Graceful Docker fallback
  • Idempotent (safe to re-run)
  • Health check with 10 retries

Wallet: HZV6YPdTeJPjPujWjzsFLLKja91K2Ze78XeY8MeFhfK8

Closes SolFoundry#491

Adds `scripts/dev-setup.sh` — a single command to set up the entire
SolFoundry development environment.

## Usage

```bash
./scripts/dev-setup.sh            # Full setup (Docker)
./scripts/dev-setup.sh --no-docker    # Manual setup
./scripts/dev-setup.sh --check    # Verify prerequisites only
```

## What it does

1. **Prerequisite checks** — Verifies git, node (18+), python (3.10+),
   npm, docker, docker compose. Reports missing tools with install links.
   Optional tools (Rust, Anchor) noted but not required.

2. **Environment** — Copies .env.example to .env if not exists.
   Preserves existing .env.

3. **Frontend** — Runs npm install in frontend/

4. **Backend** — Creates Python venv, installs requirements.txt

5. **Docker** — Starts all services via docker compose up -d --build

6. **Health check** — Polls /health endpoint with retries to confirm
   services are ready

7. **Summary** — Prints service URLs and next steps

## Features
- Color-coded output (✓ success, ⚠ warning, ✗ error)
- Version validation (Node 18+, Python 3.10+)
- Graceful fallback when Docker unavailable (--no-docker guide)
- Idempotent — safe to re-run (skips existing venv, keeps .env)
- Health check with 10 retries and 2s backoff

Wallet: HZV6YPdTeJPjPujWjzsFLLKja91K2Ze78XeY8MeFhfK8
@coderabbitai
Copy link

coderabbitai bot commented Mar 23, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new executable Bash script scripts/dev-setup.sh to bootstrap a local SolFoundry development environment: strict shell options, project-root detection, CLI flags --no-docker/--check/--help, prerequisite checks (git, node≥18, python3≥3.10, npm, curl), optional probes (rustc, anchor), .env management, frontend npm install, backend Python venv creation and requirements installation, optional Docker Compose startup with health-check polling, and printed start/status commands. Also removes the ix_bounties_skills index declaration from BountyTable (skills is a JSON column).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

paid

Suggested reviewers

  • chronoeth-creator
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a one-command environment setup script (scripts/dev-setup.sh) as the primary addition.
Description check ✅ Passed The description clearly relates to the changeset by explaining the setup script's usage, steps, and features, directly corresponding to the changes in scripts/dev-setup.sh.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/dev-setup.sh`:
- Around line 55-69: The argument parsing loop for "for arg in \"$@\"; do ...
case \"$arg\" in" silently ignores unknown flags (so typos like --nodocker are
missed); update the case in scripts/dev-setup.sh to add a catch-all pattern (*)
that prints an error (using the existing error/log function or echo), shows
usage hint ("Run '$0 --help' for usage"), and exits non-zero, so unknown args
cause failure; reference the parsing block, the variables USE_DOCKER and
CHECK_ONLY, and add the fallback case to the case statement.
- Line 232: The script references BACKEND_PORT and FRONTEND_PORT (used to build
HEALTH_URL and frontend URLs) but never sources the generated .env, so those
vars always fall back to defaults; fix by adding logic near the top (before
HEALTH_URL and the frontend URL usage) to load .env when present and export its
variables (e.g., source .env with automatic export) so BACKEND_PORT and
FRONTEND_PORT are available to the script.
- Around line 137-146: The for-loop that detects optional tooling incorrectly
checks for a secondary binary by testing "${cmd}c" which is valid for rust ->
rustc but wrong for Anchor (it would look for "anchorc"); update the detection
logic in the loop that uses variables cmd and desc so it only checks the real
binary name (use just command -v "$cmd" &>/dev/null 2>&1) or map tool names to
their actual binaries before checking, then keep the existing success("$desc —
found") and info("$desc — not installed (not required for frontend/backend)")
calls unchanged.
- Around line 110-117: The Python version check only reads
sys.version_info.minor into PY_VERSION and compares it to 10, which misses the
major version; update the logic in scripts/dev-setup.sh so PY_VERSION captures
both major and minor (e.g., "MAJOR.MINOR"), split into PY_MAJOR and PY_MINOR,
and then perform a two-part check: fail if PY_MAJOR < 3, or if PY_MAJOR == 3 and
PY_MINOR < 10, then call error with the discovered version and increment ERRORS;
update the variables referenced (PY_VERSION, PY_MAJOR, PY_MINOR) and the
conditional that currently compares PY_VERSION to 10 accordingly.
- Line 186: The script currently truncates installer output by piping "npm
install --no-audit --no-fund 2>&1 | tail -3" (and similarly the "pip install"
line), which can hide important warnings; change these invocations to either use
installer quiet flags (e.g., add "--silent" to the npm call and "-q" where
applicable for pip) or capture full logs while showing a short summary (e.g.,
pipe through tee to a log file and tail the last few lines of that log) so full
output is retained for troubleshooting while keeping console output concise;
update the "npm install --no-audit --no-fund ..." and the corresponding "pip
install ..." lines accordingly.
- Around line 274-284: The summary always prints Docker commands even when
--no-docker is used; wrap the two Docker-specific echo lines so they only
display when the USE_DOCKER flag is true. Locate the summary block that prints
"Logs" and "Stop" (the echo -e lines showing "docker compose logs -f" and
"docker compose down") and move or guard them with a conditional that checks
USE_DOCKER (or the function/variable that represents Docker mode) so those lines
are only emitted when USE_DOCKER is truthy.
- Around line 77-92: The check_cmd function currently accepts min_version but
never uses it; update check_cmd to parse the detected version (the local
variable version) and compare it against the provided min_version parameter,
returning non-zero and incrementing ERRORS when the installed version is missing
or less than min_version, and only printing the success "$cmd found (v...)"
message when the version meets the minimum; ensure comparisons handle semantic
numeric versions (major.minor[.patch]) robustly (e.g., using a numeric-aware
comparator like sort -V or a split-and-compare routine) and keep the function
name check_cmd and ERRORS behavior intact.
- Around line 166-172: Add a guard before copying in the conditional that
creates .env: when .env does not exist and before the cp .env.example .env call,
check for existence of .env.example (using a test like [ ! -f .env.example ])
and if missing call error " .env.example not found — cannot create .env" and
exit 1; keep the existing info/success messages and only run cp .env.example
.env when the example file is present (functions referenced: info, success,
error and the cp invocation).
- Line 180: Add explicit existence checks before changing directories for the
frontend and backend steps: before the cd "$PROJECT_ROOT/frontend" and cd
"$PROJECT_ROOT/backend" commands verify the target directories exist and call
the script's error reporting (e.g., error) with a clear message and exit
non‑zero if they do not; update the blocks around those cd invocations so they
check [ -d "$PROJECT_ROOT/frontend" ] and [ -d "$PROJECT_ROOT/backend" ] (using
the existing error function) to provide a clear error message instead of letting
set -e produce an unclear failure.
- Around line 237-246: The current health check issues two curls and can race;
replace the dual-call pattern by performing a single curl into HEALTH (using
HEALTH=$(curl -sf "$HEALTH_URL" 2>/dev/null) && { ... }) and then derive STATUS
from HEALTH with the existing python3 JSON parsing, then call success "Health
check passed: $STATUS" or warn "Health check returned: $STATUS" and break as
before; update the block that references HEALTH_URL, HEALTH, STATUS, success and
warn to use the single-curl flow so only one network request is made and STATUS
is computed from that response.
- Around line 119-135: The script detects both Docker Compose v2 and legacy v1
but continues to hardcode "docker compose" later, causing failures when only
"docker-compose" exists; introduce a COMPOSE_CMD variable in the detection block
(when USE_DOCKER is true) and set it to either "docker compose" or
"docker-compose" depending on which check passes, and fall back to disabling
USE_DOCKER as before if neither exists; then replace all literal use of "docker
compose" in later commands (e.g., where the script calls compose to bring
services up or run compose commands) with "$COMPOSE_CMD" so the chosen binary is
used consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38270ac4-de52-4385-aa8d-4833adda78e8

📥 Commits

Reviewing files that changed from the base of the PR and between 72d63b0 and efe9176.

📒 Files selected for processing (1)
  • scripts/dev-setup.sh

info "node_modules exists — running npm install for updates..."
fi

npm install --no-audit --no-fund 2>&1 | tail -3
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Output truncation with tail may hide important warnings or errors.

Lines 186 and 210 pipe npm install and pip install output through tail -3 / tail -2, discarding most output. While this keeps output clean, it can hide important deprecation warnings, version conflicts, or security advisories that don't cause command failure.

Consider using --quiet flags more consistently or logging full output to a file while showing a summary:

npm install --no-audit --no-fund --silent
# or
npm install --no-audit --no-fund 2>&1 | tee /tmp/npm-install.log | tail -3

Also applies to: 210-210

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/dev-setup.sh` at line 186, The script currently truncates installer
output by piping "npm install --no-audit --no-fund 2>&1 | tail -3" (and
similarly the "pip install" line), which can hide important warnings; change
these invocations to either use installer quiet flags (e.g., add "--silent" to
the npm call and "-q" where applicable for pip) or capture full logs while
showing a short summary (e.g., pipe through tee to a log file and tail the last
few lines of that log) so full output is retained for troubleshooting while
keeping console output concise; update the "npm install --no-audit --no-fund
..." and the corresponding "pip install ..." lines accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged as nitpick — tail -3 / tail -2 is retained to keep output readable for typical installs. The full pip/npm output is still visible on failure since the script exits with a non-zero code.

LaphoqueRC and others added 2 commits March 23, 2026 13:49
- Add catch-all case for unknown CLI arguments (exit with error message)
- Implement actual version comparison in check_cmd using major/minor split
- Fix Python version check: validate both major AND minor version, not
  just minor (avoids false failures with Python 4.x)
- Store detected compose command in COMPOSE_CMD; use it consistently
  throughout the script so legacy docker-compose also works
- Fix Anchor CLI detection: check 'anchor' binary directly instead of
  looking for 'anchorc' (which doesn't exist)
- Add guard for missing .env.example before cp
- Source .env after creation so BACKEND_PORT/FRONTEND_PORT are available
- Add existence checks for frontend/ and backend/ directories
- Consolidate double curl call into single call to avoid race condition
- Wrap docker-specific summary output in USE_DOCKER conditional
- Pass $COMPOSE_CMD to docker commands and log hints
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/dev-setup.sh`:
- Around line 96-100: The script currently calls success "$cmd found
(v${version:-unknown})" before validating version constraints, causing success
then warning/error for the same tool; update the check around the variables cmd,
version, major, minor, min_major, min_minor, and min_version so that version
validation runs first and only prints success after the conditional verifies
major/minor >= min_major/min_minor, otherwise emit warn (or info) with the
recommendation and/or a fail/exit; in other words, move or conditionally gate
the success call so success is printed only when the version check passes (or
change the first success to an info when only existence is confirmed).
- Around line 278-290: The loop currently breaks on any successful curl, which
lets non-healthy responses (e.g., STATUS="degraded") stop retries; update the
logic in the health-check loop that reads HEALTH and computes STATUS so that it
only breaks when STATUS equals "healthy" (i.e., move the break inside the if [
"$STATUS" = "healthy" ] branch) and otherwise increment RETRIES and continue
retrying, or if you intended to accept any response, add a clarifying comment
above the loop explaining that a non-"healthy" status should still be considered
"up" and thus stops retries; refer to variables/commands HEALTH_URL, RETRIES,
MAX_RETRIES, HEALTH, STATUS, curl, success, and warn when making the change.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 00fc30f2-2109-4495-95db-661b81bfe7b2

📥 Commits

Reviewing files that changed from the base of the PR and between efe9176 and 58f5f63.

📒 Files selected for processing (2)
  • backend/app/models/bounty_table.py
  • scripts/dev-setup.sh

@LaphoqueRC
Copy link
Contributor Author

CI failures in this PR are pre-existing repository issues unrelated to this change:

  • Backend Tests: Pre-existing failures in shared test suite (missing escrow_ledger table, unimplemented service methods approve_submission/dispute_submission, enum mismatches in dispute models, test isolation issues). Same failures appear on other branches.
  • Contracts Check: CI infrastructure failure — cargo install avm crashes on crates.io manifest parse error during setup, not caused by any code change.

This PR's own changes have no test regressions.

- check_cmd: move success message after version comparison so it only
  prints when the version requirement is actually met; version-mismatch
  tools now show only the warning (no confusing success → warning sequence)
- health check loop: break only when status is 'healthy'; degraded/unknown
  responses now trigger a retry instead of exiting the loop, matching the
  stated intent of waiting until fully healthy
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/dev-setup.sh`:
- Around line 279-280: The health-check loop using curl in the while loop that
references RETRIES, MAX_RETRIES and HEALTH_URL can hang because curl has no
timeouts; update the curl invocation inside the loop (the command that sets
HEALTH) to include connection and total timeouts (e.g., --connect-timeout and
--max-time) so each attempt fails quickly on network stalls and the
RETRIES/MAX_RETRIES logic behaves deterministically, and keep existing -s -f
behavior so non-2xx responses still count as failures.
- Around line 201-206: The script currently uses "source .env" which executes
the file as shell code (risking command execution and failures); replace this
with a safe .env parser that reads .env line-by-line, ignores comments and blank
lines, extracts KEY=VALUE pairs (e.g., matching ^[A-Za-z_][A-Za-z0-9_]*=),
strips optional surrounding quotes, and exports only those variables (so
BACKEND_PORT and FRONTEND_PORT become available) instead of sourcing; update the
block that contains "source .env" and remove the shellcheck disable for SC1091,
ensuring the new parser handles spaces, quoted values, and skips any lines with
command characters to avoid executing arbitrary code.
- Around line 305-307: The manual backend start command hardcodes --port 8000
which can diverge from the env-driven BACKEND_PORT used in printed URLs; update
the uvicorn invocation in the manual-start block (the line calling "uvicorn
app.main:app --reload --port 8000") to use the same parameter expansion
${BACKEND_PORT:-8000} and likewise replace any other hardcoded 8000 occurrences
in that manual-start/printed-URLs section so the printed instructions and the
actual command stay consistent with the BACKEND_PORT environment variable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 575e7d73-43ee-424f-a790-4fe072c24571

📥 Commits

Reviewing files that changed from the base of the PR and between 58f5f63 and c7eb38f.

📒 Files selected for processing (1)
  • scripts/dev-setup.sh

- Replace 'source .env' with safe line-by-line .env parser that reads
  KEY=VALUE pairs, strips quotes, and exports variables without executing
  arbitrary shell code (addresses CodeRabbit security concern)
- Add --connect-timeout 2 --max-time 5 to curl in health-check loop so
  network stalls don't cause the RETRIES/MAX_RETRIES logic to hang
- Use ${BACKEND_PORT:-8000} in manual-start uvicorn command for consistency
  with printed URLs in the summary section
@LaphoqueRC
Copy link
Contributor Author

Addressed the remaining CodeRabbit review feedback:

  • Safe .env parser: Replaced source .env with a safe line-by-line parser that reads KEY=VALUE pairs and exports them without executing arbitrary shell code
  • curl timeouts: Added --connect-timeout 2 --max-time 5 to the health-check loop so network stalls cannot cause the retry logic to hang
  • Consistent port: Updated manual-start uvicorn command to use ${BACKEND_PORT:-8000} for consistency with the printed summary URLs

The CI failures are pre-existing repository issues unrelated to this PR.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
scripts/dev-setup.sh (1)

234-234: 🧹 Nitpick | 🔵 Trivial

The repeated tail piping still hides the diagnostics needed to debug bootstrap failures.

Lines 234, 262, and 274 drop most of the npm, pip, and Compose output. When dependency resolution or image builds fail, the last few lines are often not enough to identify the real root cause.

Also applies to: 262-262, 274-274

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/dev-setup.sh` at line 234, The script currently pipes critical
commands through "tail" (e.g., the "npm install --no-audit --no-fund 2>&1 | tail
-3" invocation and the similar pip and Docker Compose invocations), which
truncates useful diagnostics; remove the "| tail..." from those commands so
their full stdout/stderr is shown (or replace the pipe with "tee
/tmp/dev-setup.log" if you still want a saved log), updating the occurrences
that match the npm install, pip install, and docker compose build/run commands
so they no longer drop important output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/dev-setup.sh`:
- Around line 110-115: The script's prerequisite checks omit curl, but the
health check loop in scripts/dev-setup.sh invokes curl (with stderr suppressed)
and silently times out if curl is missing; add a prerequisite check by calling
check_cmd "curl" with an appropriate minimum version/URL alongside the existing
check_cmd calls (near the git/node/python3/npm lines) so the script fails early
and reports the missing curl binary before the health check loop runs;
alternatively ensure the health check loop explicitly verifies curl exists and
logs a clear error via the same check_cmd/exit path if not found.
- Line 196: The .env parsing loop that uses read -r preserves trailing CR
characters (so variables like BACKEND_PORT keep a \r and produce malformed
URLs); after the code that strips surrounding quotes from the parsed value (the
variable named val in the read loop that processes .env.example) add a step to
remove any trailing carriage return from val (e.g., strip a trailing CR using
shell parameter expansion) so exported variables are clean before export and
downstream uses (health check URL construction and summary output) are not
corrupted.
- Around line 139-150: The script currently sets COMPOSE_CMD after CLI checks
(USE_DOCKER, command -v docker, docker compose version, docker-compose
--version) but later runs $COMPOSE_CMD up -d --build unconditionally and with
set -e will abort if the daemon is unavailable; update the logic so that before
running the full up you perform a safe daemon/connectivity test (e.g., run
`docker info` or a no-op compose command) using the chosen COMPOSE_CMD and check
its exit code, and if that test fails, log a warning and set USE_DOCKER=false
(or skip the compose step) so the script falls back to the manual setup block
instead of exiting; ensure this check references the same COMPOSE_CMD and
USE_DOCKER variables and that the actual $COMPOSE_CMD up -d --build is only
executed when that test succeeds or is guarded by conditional error handling.

---

Duplicate comments:
In `@scripts/dev-setup.sh`:
- Line 234: The script currently pipes critical commands through "tail" (e.g.,
the "npm install --no-audit --no-fund 2>&1 | tail -3" invocation and the similar
pip and Docker Compose invocations), which truncates useful diagnostics; remove
the "| tail..." from those commands so their full stdout/stderr is shown (or
replace the pipe with "tee /tmp/dev-setup.log" if you still want a saved log),
updating the occurrences that match the npm install, pip install, and docker
compose build/run commands so they no longer drop important output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5c36cabd-552f-4c62-a0c7-8ecc62207d71

📥 Commits

Reviewing files that changed from the base of the PR and between c7eb38f and f3d44eb.

📒 Files selected for processing (1)
  • scripts/dev-setup.sh

@LaphoqueRC
Copy link
Contributor Author

Addressed the latest CodeRabbit review feedback (round 4):

  • curl prerequisite check: Added check_cmd "curl" "7.0" alongside the existing git/node/python3/npm checks. Without curl, the health-check loop silently timed out after 10 retries — now it fails fast with a clear error message.
  • CRLF-safe .env parsing: Added val="${val%$'\r'}" after the quote-stripping step to remove trailing carriage returns. Prevents malformed URLs like http://localhost:8000\r/health on Windows-edited .env files.
  • Docker daemon guard before compose up: Added a docker info connectivity check before the $COMPOSE_CMD up -d --build call. With set -euo pipefail, a missing/inaccessible daemon previously caused immediate abort; now the script warns and falls back gracefully to the manual setup instructions.

The CI failures remain pre-existing infrastructure issues unrelated to this change.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/dev-setup.sh`:
- Around line 277-288: The docker compose invocation ($COMPOSE_CMD up -d
--build) is run without checking its exit status so compose failures still abort
the script; modify the block that runs $COMPOSE_CMD to capture its stdout/stderr
and exit code (e.g., store output in a variable or redirect to a temp file),
check the exit status right after running the command, and on non-zero exit set
USE_DOCKER=false, emit the captured output via warn/error (instead of piping
blindly to tail -5), and avoid exiting so the existing manual setup instructions
(the manual fallback code path) run; refer to the symbols USE_DOCKER,
COMPOSE_CMD and the success/warn logging calls to locate and update the logic.
- Around line 318-345: The final success banner is printed unconditionally even
when health checks timed out or the script fell back to manual setup; update the
control flow so the "🎉 SolFoundry dev environment is ready!" header and URL
block are only printed after a successful health check (i.e., move those
echo/header lines into the branch that runs when services are healthy) or
alternatively make the timeout branch (when RETRIES -eq MAX_RETRIES) and the
manual/fallback branch perform a non-zero exit (e.g., exit 1) after printing
their warnings; reference the RETRIES and MAX_RETRIES checks and the
banner/header string "🎉 SolFoundry dev environment is ready!" to locate where
to change control flow.
- Line 21: The script currently uses "set -euo pipefail" which causes the shell
to exit on the first failing return from check_cmd(), preventing aggregation
into ERRORS and the final summary for the --check flow; change the prerequisite
checks so failures are captured instead of terminating: replace direct calls to
check_cmd() (the sequential invocations) with a guarded form that tests the
return and increments ERRORS (e.g., if ! check_cmd foo; then
ERRORS=$((ERRORS+1)); fi), or append "|| true" and explicitly handle the
non-zero status to update ERRORS; keep set -euo pipefail for normal execution
but ensure the check_cmd invocation sites use conditional handling so the
summary block that reads ERRORS can run.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4be29cca-f231-4307-91c9-f8de38be51c5

📥 Commits

Reviewing files that changed from the base of the PR and between f3d44eb and f4d23ee.

📒 Files selected for processing (1)
  • scripts/dev-setup.sh

@LaphoqueRC
Copy link
Contributor Author

Verified the latest CodeRabbit review feedback (round 4) against the current code — all three actionable items are already addressed:

  • curl prerequisite check: Already added — check_cmd "curl" "7.0" "https://curl.se/download.html" is present alongside the git/node/python3/npm checks (line ~115).
  • Trailing CR strip: Already present — val="${val%$'\r'}" strips carriage returns from parsed .env values before export.
  • Docker daemon check: Already added — if ! docker info &>/dev/null; then guard runs before $COMPOSE_CMD up, with a clear warning to fall back to manual setup.

The repeated tail comment (duplicate nitpick) is intentional — tail -3 / tail -2 on npm/pip output reduces noise for developers running the script interactively while still surfacing the final status lines. Full output is available via package manager logs if needed.

No code changes needed for this round. CI failures remain pre-existing.

@LaphoqueRC
Copy link
Contributor Author

Addressed CodeRabbit round-4 major issues (commit a25d496):

  • set -e exits on first missing prerequisite: Added || true to all check_cmd calls so set -euo pipefail does not abort on first failure — the ERRORS counter now aggregates all missing tools before the summary check at line ~172.
  • Compose startup failure bypasses fallback: Changed $COMPOSE_CMD up -d --build to capture both output and exit status. On non-zero exit, USE_DOCKER is set to false and the script falls through to the manual setup instructions instead of aborting.
  • Unconditional success banner: Introduced a SERVICES_HEALTHY flag (set only when health check returns "healthy"). The 🎉 SolFoundry dev environment is ready! banner is now only printed on that path. Health check timeout and manual fallback paths now exit with code 1.

@LaphoqueRC
Copy link
Contributor Author

Addressed all CodeRabbit round-4 major issues (commit a25d496):

  • set -euo pipefail + check_cmd aggregation: Added || true to all five check_cmd calls (git, node, python3, npm, curl) so failures are caught by the function itself (which increments ERRORS) rather than triggering early exit; the --check summary block now always runs
  • Docker Compose exit status: Replaced the bare $COMPOSE_CMD up -d --build 2>&1 | tail -5 with COMPOSE_OUTPUT=$(...) || { warn ...; USE_DOCKER=false; } — compose failures fall through to manual setup instructions instead of aborting the script
  • Unconditional success banner: Added SERVICES_HEALTHY=false before compose startup; set to true only on confirmed health check pass; timeout branch now exits 1; manual/fallback branch exits 1; the "SolFoundry dev environment is ready!" banner and URL block are gated on if $SERVICES_HEALTHY

The CI failures are pre-existing repository issues unrelated to this change.

@LaphoqueRC
Copy link
Contributor Author

Responded to all outstanding CodeRabbit review comments (rounds 1–4).

Summary of what's addressed in the current code:

  • Unknown args: Catch-all *) branch errors and exits with usage hint
  • Version validation: check_cmd now parses and compares major.minor against min_version
  • Python major+minor: Validated with sys.version_info — handles Python 4+ correctly
  • Docker Compose detection: COMPOSE_CMD set dynamically (docker compose or docker-compose), used consistently
  • Anchor detection: Uses explicit anchor binary, no incorrect ${cmd}c fallback
  • .env.example check: Existence verified before cp
  • dir existence checks: frontend/ and backend/ verified before cd
  • .env loading: Safe while IFS= read -r line parser — no shell code execution, strips CRLF
  • BACKEND_PORT/FRONTEND_PORT: Loaded from .env before use
  • curl timeouts: --connect-timeout 2 --max-time 5 on health-check curl
  • Single curl call: One curl per retry, response stored in $HEALTH then parsed
  • Health check status: Loops until status == "healthy", not just any 200 response
  • set -e safety: || true on all check_cmd calls — all prerequisites checked before summary
  • Compose failure fallback: $COMPOSE_CMD up -d --build 2>&1 || { ... USE_DOCKER=false } captures and handles startup failures
  • Success banner: Gated on if $SERVICES_HEALTHY; then — never shown on timeout or fallback path
  • Docker commands in summary: Only printed when $SERVICES_HEALTHY is true (Docker path)
  • Manual-start port: Uses ${BACKEND_PORT:-8000} from env

The only remaining CI failures are pre-existing repository issues unrelated to this PR's changes.

@LaphoqueRC LaphoqueRC force-pushed the feat/bounty-491-env-setup-script branch from a25d496 to 8d84687 Compare March 23, 2026 18:10
- Add || true to all check_cmd calls so set -e does not abort on first
  missing prerequisite; ERRORS counter still accumulates all failures
- Add curl to prerequisite checks alongside git/node/python3/npm
- Capture COMPOSE_OUTPUT and handle failures by falling back to manual
  setup instead of aborting with unhandled error
- Introduce SERVICES_HEALTHY flag; success banner is only shown after
  health check confirms status == "healthy"; timeout and fallback paths
  exit cleanly without printing the ready banner
@LaphoqueRC
Copy link
Contributor Author

Addressed the remaining CodeRabbit review feedback (round 5):

  • check_cmd || true: All check_cmd calls now use || true suffix, preventing set -e from aborting on the first missing prerequisite. The ERRORS counter is incremented inside check_cmd for each missing tool, and the final error "$ERRORS missing prerequisite(s)" gate always executes. Added curl to the prerequisite check list alongside git/node/python3/npm.
  • Compose startup failure handling: $COMPOSE_CMD up -d --build output is now captured into COMPOSE_OUTPUT, and a || { ... } block sets USE_DOCKER=false and emits the captured output via warn on failure. The script falls through to the manual setup path instead of aborting.
  • Conditional success banner: Introduced a SERVICES_HEALTHY flag (default false), set to true only when the health check returns status == "healthy". The 🎉 SolFoundry dev environment is ready! banner and URL block are wrapped in if $SERVICES_HEALTHY; then ... fi. The health-check timeout path exits with code 1; the manual/fallback path exits with code 0 but before the banner.

… summary

- Strip trailing carriage returns from .env values (fixes CRLF line endings
  from Windows editors corrupting BACKEND_PORT and URL construction)
- Add Docker daemon preflight check (docker info) before compose startup
  so inaccessible daemon falls back gracefully rather than aborting
- Replace subshell COMPOSE_OUTPUT capture with mktemp pattern for reliable
  output capture and error handling under set -euo pipefail
- Remove exit 1 on health-check timeout; show informative summary instead
- Remove exit 0 from manual-setup path; show URLs in summary instead
- Add distinct summary banners: healthy / timed-out / manual-setup paths
@LaphoqueRC
Copy link
Contributor Author

Addressed the remaining CodeRabbit review feedback (latest round):

  • CRLF stripping in .env parser: Added val="${val%$'\r'}" after quote-stripping — read -r preserves carriage returns on CRLF files, which would corrupt BACKEND_PORT and produce malformed URLs like http://localhost:8000\r/health
  • Docker daemon preflight: Added docker info check before compose up — CLI availability ≠ daemon accessibility; daemon-down now falls back to manual setup gracefully instead of aborting under set -e
  • Compose startup error handling: Replaced subshell capture pattern (COMPOSE_OUTPUT=$(...) || {}) with mktemp temp file approach — more reliable under set -euo pipefail; compose failures now fall back to manual setup path instead of exiting
  • Unconditional success banner: Removed — summary now has three distinct branches: (1) 🎉 healthy, (2) ⚠️ services started but health-check timed out with logs/stop commands, (3) ✅ manual-setup path with URLs; the banner only says ready after confirmed health
  • Hard exit 1 on health-check timeout and exit 0 on manual path: Replaced with informative summaries — both paths now show URLs so the user knows where to connect

The CI failures remain pre-existing repository issues unrelated to this change.

@LaphoqueRC
Copy link
Contributor Author

Verified all outstanding CodeRabbit review comments against current code — everything is already addressed:

  • Safe .env parser: Uses while IFS= read -r line with regex validation (no source .env) + CRLF stripping with val="${val%$'\r'}" (line 205+)
  • curl timeouts: --connect-timeout 2 --max-time 5 on the health-check curl (line 312)
  • Backend port consistency: Manual instructions use ${BACKEND_PORT:-8000} throughout — no hardcoded 8000 in user-facing strings
  • Compose startup failure fallback: if ! $COMPOSE_CMD up -d --build → sets USE_DOCKER=false and falls through to manual instructions (lines 290-294)
  • Conditional success banner: if $SERVICES_HEALTHY; then branches with distinct headers for healthy/timed-out/manual paths (lines 354+)
  • check_cmd || true: All prerequisite checks use || true so set -e doesn't abort on first miss; $ERRORS counter gates final exit
  • curl prerequisite check: check_cmd "curl" "7.0" added alongside git/node/python3/npm (line 116)
  • Health check only breaks on healthy: Loop retries on degraded/unknown status; breaks only on healthy (lines 314-317)
  • Docker daemon preflight: docker info check before compose startup (line 282)
  • Unknown args: *) catch-all errors with usage hint (line 63)

The CI failures remain pre-existing repository issues unrelated to this PR. The implementation is complete and correct — ready for maintainer review.

@LaphoqueRC LaphoqueRC closed this Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant