Skip to content

better run options#469

Merged
tcsenpai merged 1 commit intotestnetfrom
better_run
Sep 14, 2025
Merged

better run options#469
tcsenpai merged 1 commit intotestnetfrom
better_run

Conversation

@tcsenpai
Copy link
Contributor

@tcsenpai tcsenpai commented Sep 14, 2025

PR Type

Enhancement


Description

  • Enhanced run script with comprehensive system requirements checking

  • Added verbose logging and improved user experience with emojis

  • Created standalone database manager script for external access

  • Improved error handling and cross-platform compatibility


Diagram Walkthrough

flowchart LR
  A["run script"] --> B["System Requirements Check"]
  B --> C["Docker & Dependencies Check"]
  C --> D["PostgreSQL Startup"]
  D --> E["Node Startup"]
  F["start_db script"] --> G["Standalone Database"]
  G --> H["External Tool Access"]
Loading

File Walkthrough

Relevant files
Enhancement
run
Enhanced run script with system validation                             

run

  • Added comprehensive system requirements validation (RAM, CPU, network,
    ports)
  • Enhanced user interface with emojis, better formatting, and verbose
    logging
  • Improved error handling and cross-platform compatibility (macOS/Linux)
  • Added help documentation and better command-line argument handling
+362/-51
start_db
New standalone database manager script                                     

start_db

  • Created new standalone database manager script
  • Provides isolated PostgreSQL access for external tools
  • Includes help documentation and verbose logging
  • Supports clean database initialization and port configuration
+261/-0 

Summary by CodeRabbit

  • New Features
    • Optional verbose mode and expanded CLI flags (-v, -h) with built-in help.
    • Cross-platform behavior (macOS/Linux) with platform-aware checks and guidance.
    • Comprehensive system requirements and dependency validation with actionable messages.
    • Default runtime migrates to Bun when available, with clear fallbacks.
    • Per-port PostgreSQL isolation to run multiple nodes concurrently.
    • New database launcher script to start/stop a per-port PostgreSQL via Docker.
  • Improvements
    • Clearer startup/restore flows, richer status messages, and consistent output.
    • More robust readiness waits, graceful shutdowns, and final session summaries.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 14, 2025

Caution

Review failed

The pull request is closed.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch better_run

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0a30fb7 and 324a870.

📒 Files selected for processing (2)
  • run (9 hunks)
  • start_db (1 hunks)

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.

❤️ Share

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

@tcsenpai tcsenpai merged commit 15fca71 into testnet Sep 14, 2025
1 check was pending
@tcsenpai tcsenpai deleted the better_run branch September 14, 2025 18:55
@qodo-code-review
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive information exposure:
The start_db script prints database credentials (username and password) in clear text to the console. While this may be intentional for local development, it could leak in shared logs or CI outputs. Consider masking or providing an option to suppress credentials, and remind users to treat them as development-only.

⚡ Recommended focus areas for review

Possible Issue

Port check uses $PORT before it is guaranteed to be initialized in check_system_requirements, which may yield empty or misleading results on first runs.

# Check port availability
log_verbose "Checking port availability"
if command -v lsof > /dev/null; then
    if lsof -i :$PG_PORT > /dev/null 2>&1; then
        echo "❌ PostgreSQL port $PG_PORT is already in use"
        failed_requirements=$((failed_requirements + 1))
    else
        echo "✅ PostgreSQL port $PG_PORT is available"
    fi

    if lsof -i :$PORT > /dev/null 2>&1; then
        echo "❌ Node port $PORT is already in use"
        failed_requirements=$((failed_requirements + 1))
    else
        echo "✅ Node port $PORT is available"
    fi
elif command -v netstat > /dev/null; then
Robustness

ping parsing for latency relies on awk -F'/' '{print $5}' | cut -d'.' -f1, which can fail on locales or different ping outputs; consider more robust parsing and integer validation.

log_verbose "Checking network connectivity"
if command -v ping > /dev/null; then
    if [ "$PLATFORM_NAME" = "macOS" ]; then
        # macOS ping syntax
        ping_result=$(ping -c 3 -t 5 1.1.1.1 2>/dev/null | tail -1 | awk -F'/' '{print $5}' | cut -d'.' -f1)
    elif [ "$PLATFORM_NAME" = "Linux" ]; then
        # Linux ping syntax
        ping_result=$(ping -c 3 -W 5 1.1.1.1 2>/dev/null | tail -1 | awk -F'/' '{print $5}' | cut -d'.' -f1)
    fi

    if [ -z "$ping_result" ]; then
        echo "❌ Network connectivity failed - cannot reach 1.1.1.1"
        failed_requirements=$((failed_requirements + 1))
    elif [ "$ping_result" -gt 200 ]; then
        echo "❌ Network latency too high: ${ping_result}ms (maximum: 200ms)"
        failed_requirements=$((failed_requirements + 1))
    elif [ "$ping_result" -gt 100 ]; then
        echo "⚠️  Network latency above recommended: ${ping_result}ms (recommended: <100ms)"
        warnings=$((warnings + 1))
    else
        echo "✅ Network latency: ${ping_result}ms"
    fi
Port Probe

Port availability check via netstat | grep ":$PG_PORT " may miss matches on systems with different formatting; consider using ss or stricter regex to avoid false negatives/positives.

# Check port availability
log_verbose "Checking port availability"
if command -v lsof > /dev/null; then
    if lsof -i :$PG_PORT > /dev/null 2>&1; then
        echo "❌ PostgreSQL port $PG_PORT is already in use"
        echo "💡 Use -d flag to specify a different port, or stop the existing service"
        exit 1
    else
        echo "✅ PostgreSQL port $PG_PORT is available"
    fi
elif command -v netstat > /dev/null; then
    if netstat -an | grep ":$PG_PORT " > /dev/null 2>&1; then
        echo "❌ PostgreSQL port $PG_PORT is already in use"
        echo "💡 Use -d flag to specify a different port, or stop the existing service"
        exit 1
    else
        echo "✅ PostgreSQL port $PG_PORT is available"
    fi
else
    echo "⚠️  Cannot check port availability - lsof and netstat not available"
fi

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consolidate scripts to avoid duplication

The start_db script duplicates a lot of logic from the run script (e.g.,
platform detection, Docker checks). To improve maintainability, this
functionality should be consolidated. One approach is to merge start_db's
purpose into the run script, accessible via a new command-line flag like
--db-only.

Examples:

run [8-265]
# Detect platform for cross-platform compatibility
PLATFORM=$(uname -s)
case $PLATFORM in
    "Darwin")
        PLATFORM_NAME="macOS"
        ;;
    "Linux")
        PLATFORM_NAME="Linux"
        ;;
    *)

 ... (clipped 248 lines)
start_db [7-161]
# Detect platform for cross-platform compatibility
PLATFORM=$(uname -s)
case $PLATFORM in
    "Darwin")
        PLATFORM_NAME="macOS"
        ;;
    "Linux")
        PLATFORM_NAME="Linux"
        ;;
    *)

 ... (clipped 145 lines)

Solution Walkthrough:

Before:

# File: run
#!/bin/bash
PLATFORM=$(uname -s)
# ... platform detection logic ...

check_system_requirements() {
  # ... checks for RAM, CPU, network, ports ...
}
check_docker() { ... }

check_system_requirements
check_docker
# ... start database ...
# ... start node application ...

# File: start_db
#!/bin/bash
PLATFORM=$(uname -s)
# ... (duplicated) platform detection logic ...

check_docker() { ... } # (duplicated)
check_port_availability() { ... } # (duplicated)

check_docker
check_port_availability
# ... (duplicated) start database logic ...

After:

# File: run
#!/bin/bash
DB_ONLY="false"

while getopts "...f" opt; do # Add a new flag 'f' for db-only
    case $opt in
        f) DB_ONLY="true";;
        # ... other options
    esac
done

# ... (shared) platform detection, docker checks, db start logic ...

check_system_requirements
check_docker
start_database

if [ "$DB_ONLY" == "true" ]; then
    echo "Database started in standalone mode. Press Ctrl+C to exit."
    while true; do sleep 1; done
else
    # ... start node application ...
fi
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies significant code duplication between the run and start_db scripts, which introduces a major maintenance burden and is a structural flaw in the PR.

Medium
Possible issue
Fix incorrect command-line flag parsing

Correct the getopts string and case statement to handle the -c and -n options as
boolean flags instead of options requiring arguments.

run [284-299]

-while getopts "p:d:c:i:n:u:l:r:b:vh" opt; do
+while getopts "p:d:ci:nu:l:r:b:vh" opt; do
      case $opt in
          p) PORT=$OPTARG;;
          d) PG_PORT=$OPTARG;;
-         c) CLEAN=$OPTARG;;
+         c) CLEAN="true";;
          i) IDENTITY_FILE=$OPTARG;;
          l) PEER_LIST_FILE=$OPTARG;;
          n) GIT_PULL=false;;
          u) EXPOSED_URL=$OPTARG;;
          r) RUNTIME=$OPTARG;;
          b) RESTORE=$OPTARG;;
          v) VERBOSE=true;;
          h) show_help; exit 0;;
          *) echo "Invalid option. Use -h for help."; exit 1;;
      esac
  done
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug in command-line argument parsing for the -c and -n flags, which would cause unexpected behavior and errors for users.

Medium
General
Remove redundant database cleanup command

Remove the redundant rm -rf data_${PG_PORT} command from the database cleanup
logic to improve code clarity.

run [419-422]

 if [ "$CLEAN" == "true" ]; then
-    rm -rf data_${PG_PORT} || rm -rf data_${PG_PORT}
+    rm -rf data_${PG_PORT}
     mkdir data_${PG_PORT}
 fi

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion correctly identifies a redundant and confusing command, and removing it improves code quality and readability without affecting functionality.

Low
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant