From 346dfd495f8cc0be7759d94f8ab3329f6a4a1b1e Mon Sep 17 00:00:00 2001 From: Linda Goldstein Date: Mon, 7 Sep 2026 22:41:54 -0700 Subject: [PATCH 1/2] Make bin/setup fail fast, fail clearly, and stop dropping the dev DB bin/setup had two control-flow bugs, destroyed local data on every run, and surfaced its most common failure several minutes in. Bugs: - `system!('bundle check') || system!('bundle install')` could never fall through. `system!` passes `exception: true`, so a failing `bundle check` raised instead of reaching the fallback -- meaning every fresh clone, the one case the line exists for, crashed instead of installing gems. Use the non-raising `try_system` for commands whose failure is a branch rather than an error (bin/update already had this right). Same dead `|| abort` after `npm ci`. - The Ruby version mismatch path called bare `exit`, which exits 0. Callers such as .devcontainer/post-create.sh read that as success, so a Codespace came up with nothing installed and no error. Now aborts non-zero. Data loss: - `db:reset` ran unconditionally, dropping the development database, while the file's own comment claimed idempotency. Default to `db:prepare`; keep the destructive path behind an explicit `--reset` (or FORCE_DB_RESET=1), which is what the devcontainer now passes. Fail fast: - Move all cheap validation ahead of the first `gem install`: .env creation, a warning for keys in .env.example missing from .env, an exact Ruby version comparison, a Node presence and major-version check against engines.node, and a real postgres connection attempt using the .env values. Errors name the variable to edit instead of printing a warning emoji and pressing on. The checks are stdlib-only since they run before `bundle install`. Also drops the redundant `gem install foreman` (bin/dev installs it on demand) and the no-op `bin/rails restart` on a fresh clone, adds `--skip-assets` and `--help`, unbuffers stdout so piped logs stay ordered, and ends with the seed login and next steps. Co-Authored-By: Claude Opus 5 --- .devcontainer/post-create.sh | 4 +- CLAUDE.md | 2 +- README.md | 24 +++- bin/setup | 210 ++++++++++++++++++++++++++++------- 4 files changed, 196 insertions(+), 44 deletions(-) diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index 7881b6c772..afe07df48b 100755 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -11,4 +11,6 @@ if [ "$RUBY_VERSION" != "4.0.6" ]; then echo "Ruby $RUBY_VERSION installed" fi -bin/setup +# --reset builds the development database from scratch. This is a brand new +# container, so there is no local data to lose. +bin/setup --reset diff --git a/CLAUDE.md b/CLAUDE.md index e17914ad3b..50a5b7f4ec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,7 +10,7 @@ CASA is a Rails app used by CASA (Court Appointed Special Advocate) chapters to | Task | Command | |---|---| -| One-time setup | `bin/setup` | +| One-time setup | `bin/setup` (safe to re-run; `--reset` to rebuild the dev DB, `--skip-assets` to skip npm) | | Run app (web + JS/CSS watchers) | `bin/dev` then visit http://localhost:3000 | | Run full RSpec suite | `bin/rails spec` | | Run a single spec file | `bundle exec rspec spec/path/to/file_spec.rb` | diff --git a/README.md b/README.md index eedce41dcf..bd3f0e0367 100644 --- a/README.md +++ b/README.md @@ -204,11 +204,19 @@ Run these commands before starting the installation process:
-bin/setup fails with a credentials error +bin/setup says it cannot connect to postgres -1. Open the `.env` file. -2. Update `POSTGRES_USER` and `POSTGRES_PASSWORD` to match your PostgreSQL credentials. -3. Run `bin/setup` +`bin/setup` checks the database before doing anything slow, and prints the host, port +and user it tried. Either postgres is not running, or the credentials in `.env` are +wrong — update `POSTGRES_USER`, `POSTGRES_PASSWORD` and `DATABASE_HOST` to match your +PostgreSQL setup and re-run `bin/setup`. +
+ +
+I want to throw away my local data and start over + +Run `bin/setup --reset`. By default `bin/setup` preserves the development database; +`--reset` drops, recreates and reseeds it.
## Running the App / Verifying Installation @@ -216,6 +224,14 @@ Run these commands before starting the installation process: 1. Run `bin/setup` 1. Run `bin/dev` and visit http://localhost:3000/ to see the app running. +`bin/setup` is safe to re-run — it preserves your development database. Options: + +| Flag | Effect | +|---|---| +| `--reset` | Drop, recreate and reseed the development database (destroys local data) | +| `--skip-assets` | Skip `npm ci` and the JS/CSS builds | +| `--help` | List the options | + ### QA Environment A publicly accessible QA environment is available at **https://casa-qa.herokuapp.com/**. You can log in using the same seed credentials below — useful for exploring the app without any local setup. diff --git a/bin/setup b/bin/setup index 4fd6653e5c..43b5b71e08 100755 --- a/bin/setup +++ b/bin/setup @@ -1,62 +1,196 @@ #!/usr/bin/env ruby -require 'fileutils' +require "fileutils" +require "json" +require "optparse" + +# Unbuffered, so progress and error output stay in order when this is piped to a +# log rather than a terminal (e.g. the Codespaces post-create script). +$stdout.sync = true # path to your application root. -APP_ROOT = File.expand_path('..', __dir__) +APP_ROOT = File.expand_path("..", __dir__) +# Raises on failure. Use for steps where a non-zero exit is a hard stop. def system!(*args) system(*args, exception: true) end -FileUtils.chdir APP_ROOT do - # This script is a way to set up or update your development environment automatically. - # This script is idempotent, so that you can run it at any time and get an expectable outcome. - # Add necessary setup steps to this file. - expected_ruby_version = `cat .ruby-version`.chomp - current_ruby_version = `ruby -v`.chomp - unless current_ruby_version.include?(expected_ruby_version) - puts "Ruby version must be #{expected_ruby_version}. You are on #{current_ruby_version}" +# Returns false on failure. Use where a non-zero exit is a branch, not an error +# (e.g. `bundle check` failing just means we need to `bundle install`). +def try_system(*args) + system(*args) +end + +def abort_with(message) + abort "\n#{message}\n" +end + +def step(title) + puts "\n== #{title} ==" +end + +# Parse KEY=value lines out of a dotenv-style file. Hand-rolled because this +# runs before `bundle install`, so the dotenv gem is not available yet. +def parse_env_file(path) + return {} unless File.exist?(path) + + File.readlines(path).each_with_object({}) do |line, env| + next if line.strip.empty? || line.strip.start_with?("#") + + key, _, value = line.strip.partition("=") + next if key.empty? + + env[key] = value.gsub(/\A["']|["']\z/, "") + end +end + +options = {reset: ENV["FORCE_DB_RESET"] == "1", assets: true} +OptionParser.new do |opts| + opts.banner = "Usage: bin/setup [options]" + opts.on("--reset", "Drop, recreate and reseed the development database (destroys local data)") { options[:reset] = true } + opts.on("--skip-assets", "Skip npm install and the JS/CSS builds") { options[:assets] = false } + opts.on("-h", "--help", "Show this message") do + puts opts exit end +end.parse! + +FileUtils.chdir APP_ROOT do + # This script sets up your development environment. It is safe to re-run: by + # default the database is prepared, not reset, so local data survives. + # Use `bin/update` after pulling main; use this script for first-time setup. + # + # Everything cheap is checked up front, so a misconfigured machine fails in + # seconds rather than after several minutes of `bundle install`. + step "Checking prerequisites" + + # .env comes first: the database check below reads its values. + unless File.exist?(".env") + puts "Creating .env from .env.example" + FileUtils.cp ".env.example", ".env" + end - puts "\n== Installing dependencies ==" - system! 'gem install foreman' + env = parse_env_file(".env") + missing_keys = parse_env_file(".env.example").keys - env.keys + if missing_keys.any? + puts "⚠️ .env is missing keys that .env.example defines: #{missing_keys.join(", ")}" + puts " Copy them over from .env.example if something behaves unexpectedly." + end + + expected_ruby_version = File.read(".ruby-version").strip + if expected_ruby_version != RUBY_VERSION + abort_with <<~MSG + Ruby #{expected_ruby_version} is required, but this is Ruby #{RUBY_VERSION}. + + With rbenv: rbenv install #{expected_ruby_version} && rbenv rehash + With rvm: rvm install #{expected_ruby_version} && rvm use #{expected_ruby_version} + With asdf: asdf install ruby #{expected_ruby_version} + MSG + end + puts "Ruby #{RUBY_VERSION} ✓" + + node_version = `node --version 2>/dev/null`.strip + abort_with <<~MSG if node_version.empty? + Node.js was not found on your PATH. + + Install nvm (https://github.com/nvm-sh/nvm), then run `nvm install` from this + directory to pick up the version in .nvmrc. + MSG + + expected_node_major = JSON.parse(File.read("package.json")).dig("engines", "node").to_s[/\d+/] + actual_node_major = node_version[/\d+/] + if expected_node_major && actual_node_major != expected_node_major + abort_with <<~MSG + Node.js #{expected_node_major}.x is required, but this is #{node_version}. + + Run `nvm install` from this directory to install the version in .nvmrc. + MSG + end + puts "Node #{node_version} ✓" + + # Confirm postgres is up AND that the credentials in .env work. This is the + # single most common setup failure, so it is worth catching before the slow + # steps rather than surfacing it as a Rails connection error later. + db_host = env.fetch("DATABASE_HOST", "localhost") + db_port = env.fetch("POSTGRES_PORT", "5432") + db_user = env["POSTGRES_USER"].to_s + db_connection_hint = <<~HINT + Checked host=#{db_host} port=#{db_port} user=#{db_user.empty? ? "(unset)" : db_user}, from your .env file. + + - Not running? Start postgres (`brew services start postgresql` on macOS), or see + doc/DOCKER.md if you run it in a container. + - Wrong credentials? Update POSTGRES_USER / POSTGRES_PASSWORD / DATABASE_HOST in .env. + HINT + + if try_system("command -v psql > /dev/null 2>&1") + # psql validates credentials, not just reachability. + psql_env = {"PGPASSWORD" => env["POSTGRES_PASSWORD"].to_s, "PGCONNECT_TIMEOUT" => "5"} + psql_cmd = ["psql", "-h", db_host, "-p", db_port, "-U", db_user, "-d", "postgres", "-c", "select 1"] + unless try_system(psql_env, *psql_cmd, out: File::NULL, err: File::NULL) + abort_with "Could not connect to postgres.\n\n#{db_connection_hint}" + end + puts "PostgreSQL connection ✓" + elsif try_system("command -v pg_isready > /dev/null 2>&1") + unless try_system("pg_isready", "-h", db_host, "-p", db_port, out: File::NULL, err: File::NULL) + abort_with "PostgreSQL is not accepting connections.\n\n#{db_connection_hint}" + end + puts "PostgreSQL is accepting connections ✓ (install psql to also verify credentials)" + else + puts "⚠️ Neither psql nor pg_isready found; skipping the database connection check." + end + + step "Installing dependencies" # Pin bundler to a version Heroku supports for our Ruby. Match Heroku's # supported Ruby/Bundler pairings as they evolve: # https://devcenter.heroku.com/articles/ruby-support-reference#supported-ruby-versions - system! 'gem install bundler -v 4.0.6 --conservative' - system!('bundle check') || system!('bundle install') + system! "gem install bundler -v 4.0.6 --conservative" + try_system("bundle check") || system!("bundle install") - # puts '\n== Copying sample files ==' - # unless File.exist?('config/database.yml') - # FileUtils.cp 'config/database.yml.sample', 'config/database.yml' - # end - - unless File.exist?('.env') - puts "\n== Setup .env file from .env.example ==" - system!('cp .env.example .env') + if options[:reset] + step "Resetting database" + puts "Dropping, recreating and reseeding the development database." + system! "bin/rails db:reset" + else + step "Preparing database" + puts "Creating and seeding the database if needed; existing local data is preserved." + puts "Pass --reset to drop and reseed from scratch." + system! "bin/rails db:prepare" end + system! "bin/rails db:test:prepare" + + step "Removing old logs and tempfiles" + system! "bin/rails log:clear tmp:clear" - puts "\n== Preparing database ==" - puts '⚠️ If you use docker to run postgres, make sure your database is running ⚠️' - system! 'bin/rails db:reset' - system! 'bin/rails db:test:prepare' + step "Running post-deployment tasks" + system! "bin/rake after_party:run" - puts "\n== Removing old logs and tempfiles ==" - system! 'bin/rails log:clear tmp:clear' + if options[:assets] + step "Installing npm packages" + unless try_system("npm ci") + abort_with <<~MSG + `npm ci` failed. - puts "\n== Restarting application server ==" - system! 'bin/rails restart' + It installs strictly from package-lock.json, so this usually means the lockfile + and package.json disagree. Try `npm install` to reconcile them, and check the + output above for the offending package. + MSG + end + + step "Building assets" + system! "npm run build" + system! "npm run build:css" + end - puts "\n== Running post-deployment tasks ==" - system! 'bin/rake after_party:run' + puts <<~DONE - puts "\n== Installing npm packages ==" - system!('npm ci') || abort('install npm and try again') + == Done == - puts "\n== Building assets ==" - system!('npm run build') - system!('npm run build:css') + Next steps: + 1. bin/dev + 2. Open http://localhost:3000/users/sign_in + 3. Log in as volunteer1@example.com / 12345678 + (also supervisor1@example.com, casa_admin1@example.com — same password) - puts "\n== Done ==" + After pulling main, run bin/update rather than bin/setup. + DONE end From 142e89028a4549761352068b1cf1a3083c7a4e40 Mon Sep 17 00:00:00 2001 From: Linda Goldstein Date: Tue, 8 Sep 2026 00:11:18 -0700 Subject: [PATCH 2/2] Add a shellcheck CI job and fix the warning-level findings it caught The repo has 24 shell scripts and no shell linter. Adds a Shell lint workflow running shellcheck, and fixes the four files that kept it from being green. The workflow selects scripts by shebang rather than by path: most of them have no extension (bin/dev, docker/*), and bin/ also holds Ruby scripts, so neither a glob nor a directory list gets the set right. Gated at --severity=warning. The tree still has ~35 info/style findings, mostly SC2086 (unquoted variables) across bin/git_hooks and docker/. Those are worth fixing, but gating on them now would mean rewriting scripts unrelated to whatever PR is under review -- the same gradual-adoption stance as .standard_todo.yml. The comment in the workflow says to lower the threshold as they get cleaned up. Findings fixed: - bin/git_hooks/lint: `git diff --name-only HEAD~$(git cherry ... | wc -l)` was unquoted (SC2046), and on BSD/macOS `wc -l` pads its output with spaces -- so word splitting passed git `HEAD~` and `1` as separate arguments and the --unpushed path died with "ambiguous argument '1'". Strip the padding and quote. Also `cd ... || exit 1` in five places (SC2164), so a failed cd can no longer run a linter in the wrong directory. - docker/run: `docker compose run web $@` -> `"$@"` (SC2068). Unquoted `$@` re-splits arguments, so `docker/run rspec -e "some example"` lost its quoting. - bin/git_hooks/update-dependencies: replace the two-branch `[ $(git diff ... | wc -l) -gt 0 ]` test with a single `git diff --quiet` over both pathspecs (SC2046, SC1083). Verified equivalent for lockfile-only, package.json-only and unrelated changes. - .devcontainer/post-create.sh: add a shebang (SC2148). Co-Authored-By: Claude Opus 5 --- .devcontainer/post-create.sh | 1 + .github/workflows/shell_lint.yml | 80 +++++++++++++++++++++++++++++++ bin/git_hooks/lint | 18 +++---- bin/git_hooks/update-dependencies | 2 +- docker/run | 2 +- 5 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/shell_lint.yml diff --git a/.devcontainer/post-create.sh b/.devcontainer/post-create.sh index afe07df48b..25480cb6e5 100755 --- a/.devcontainer/post-create.sh +++ b/.devcontainer/post-create.sh @@ -1,3 +1,4 @@ +#!/usr/bin/env bash RUBY_VERSION="$(cat .ruby-version | tr -d '\n')" # copy the file only if it doesn't already exist diff --git a/.github/workflows/shell_lint.yml b/.github/workflows/shell_lint.yml new file mode 100644 index 0000000000..acf570fcd6 --- /dev/null +++ b/.github/workflows/shell_lint.yml @@ -0,0 +1,80 @@ +name: Shell lint + +on: + push: + branches: + - main + paths: + - '**/*.sh' + - '**/*.bash' + - 'bin/**' + - 'docker/**' + - '.devcontainer/**' + - '.github/workflows/shell_lint.yml' + pull_request: + branches: + - main + - rfg-event-2025 + paths: + - '**/*.sh' + - '**/*.bash' + - 'bin/**' + - 'docker/**' + - '.devcontainer/**' + - '.github/workflows/shell_lint.yml' + +jobs: + shellcheck: + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Install shellcheck if the runner image lacks it + run: | + if ! command -v shellcheck > /dev/null 2>&1; then + sudo apt-get update + sudo apt-get install --no-install-recommends -y shellcheck + fi + shellcheck --version + + - name: Shellcheck + # Many scripts here have no extension (bin/dev, docker/*), and bin/ also + # holds Ruby scripts, so select by shebang rather than by path. + # + # --severity=warning: the tree still has ~35 info/style findings, mostly + # SC2086 (unquoted variables) in bin/git_hooks and docker. Those are + # worth fixing, but gating on them today would mean rewriting scripts + # unrelated to whatever PR is being reviewed. Lower this as they get + # cleaned up. + run: | + set -eu + + list=$(mktemp) + trap 'rm -f "$list"' EXIT + + { + git ls-files -- '*.sh' '*.bash' + git ls-files | while IFS= read -r file; do + [ -f "$file" ] || continue + if head -n 1 "$file" | + grep -qE '^#!.*[ /](bash|sh|dash|ksh)$'; then + printf '%s\n' "$file" + fi + done + } | sort -u > "$list" + + count=$(wc -l < "$list" | tr -d '[:space:]') + if [ "$count" -eq 0 ]; then + echo "No shell scripts found -- check the discovery logic above." + exit 1 + fi + + echo "Checking $count shell scripts:" + sed 's/^/ /' "$list" + echo + + xargs shellcheck --severity=warning -- < "$list" diff --git a/bin/git_hooks/lint b/bin/git_hooks/lint index 3fe4ea5166..7c72c33740 100755 --- a/bin/git_hooks/lint +++ b/bin/git_hooks/lint @@ -32,10 +32,12 @@ case $diff_policy in ;; --unpushed) - if [ -z "$(git ls-remote --heads origin ${current_branch})" ]; then - changed_file_list=$(git diff --name-only HEAD~$(git cherry -v main ${current_branch} | wc -l) HEAD) + if [ -z "$(git ls-remote --heads origin "${current_branch}")" ]; then + # wc -l pads with whitespace on BSD/macOS, so strip it before interpolating. + unpushed_count=$(git cherry -v main "${current_branch}" | wc -l | tr -d '[:space:]') + changed_file_list=$(git diff --name-only "HEAD~${unpushed_count}" HEAD) else - changed_file_list=$(git diff --name-only origin/${current_branch}..HEAD) + changed_file_list=$(git diff --name-only "origin/${current_branch}..HEAD") fi ;; @@ -55,7 +57,7 @@ lint_time=$(date +%s) if test $erb_changed_count -gt 0; then log info "Linting via erblint" - cd $repo_root/app + cd "$repo_root/app" || exit 1 if ! [ -x "$(command -v bundle)" ]; then log error "Command bundle could not be found" @@ -71,7 +73,7 @@ fi if test $factory_changed_count -gt 0; then log info "Linting via factory:lint" - cd $repo_root/app + cd "$repo_root/app" || exit 1 if ! [ -x "$(command -v bundle)" ]; then log error "Command bundle could not be found" @@ -87,7 +89,7 @@ fi if test $js_changed_count -gt 0; then log info "Linting javasript via standard" - cd $repo_root/app + cd "$repo_root/app" || exit 1 if ! [ -x "$(command -v npm)" ]; then log error "Command npm could not be found" @@ -103,7 +105,7 @@ fi if test $rb_changed_count -gt 0; then log info "Linting via standardrb" - cd $repo_root + cd "$repo_root" || exit 1 if ! [ -x "$(command -v bundle)" ]; then log error "Command bundle could not be found" @@ -116,7 +118,7 @@ if test $rb_changed_count -gt 0; then fi fi -cd $repo_root +cd "$repo_root" || exit 1 for file in $(git diff --name-only); do last_modified_time=$(date -r $file +%s) if [ $last_modified_time -ge $lint_time ] ; then diff --git a/bin/git_hooks/update-dependencies b/bin/git_hooks/update-dependencies index 05ea389622..956a0537f0 100755 --- a/bin/git_hooks/update-dependencies +++ b/bin/git_hooks/update-dependencies @@ -28,7 +28,7 @@ fi log info "Checking javascript dependency status" -if [ $(git diff HEAD@{1}..HEAD@{0} -- "package-lock.json" | wc -l) -gt 0 ] || [ $(git diff HEAD@{1}..HEAD@{0} -- "package.json" | wc -l) -gt 0 ]; then +if ! git diff --quiet "HEAD@{1}..HEAD@{0}" -- package-lock.json package.json; then log info "Updating JavaScript dependencies" npm install fi \ No newline at end of file diff --git a/docker/run b/docker/run index d9bd2733e2..32c9a6e8d7 100755 --- a/docker/run +++ b/docker/run @@ -19,4 +19,4 @@ function cleanup { trap cleanup EXIT -docker compose run web $@ +docker compose run web "$@"