Make bin/setup fail fast, fail clearly, and stop dropping the dev DB

[compwron]

Why

bin/setup is the first thing a new contributor runs. Today it has two control-flow bugs, destroys local data on every run, and surfaces its single most common failure several minutes in.

Bugs fixed

1. bundle check || bundle install could never fall through.

def system!(*args) = system(*args, exception: true)
system!('bundle check') || system!('bundle install')

exception: true means a failing bundle check raises; the || is dead code. Verified:

RAISED: RuntimeError: Command failed with exit 1: false

So the one situation the line exists for — gems not installed yet, i.e. every fresh clone — crashed the script instead of installing them. bin/update:20 already had the correct non-raising form. This PR adds an explicit try_system for commands whose non-zero exit is a branch rather than an error, and uses it here and for npm ci (whose || abort was unreachable too, and whose message blamed a missing npm for what is almost always a lockfile mismatch).

2. The Ruby version mismatch path exited 0.

Bare exit reports success, so .devcontainer/post-create.sh and any CI caller treated "wrong Ruby, nothing installed" as a clean setup. Now aborts non-zero, with the install command for rbenv/rvm/asdf.

Stops dropping the development database

bin/rails db:reset ran unconditionally — directly under a comment claiming the script "is idempotent, so that you can run it at any time". Re-running setup silently destroyed local data.

• Default is now db:prepare: creates and seeds when needed, preserves what is there.
• --reset (or FORCE_DB_RESET=1) keeps the destructive path, explicitly. .devcontainer/post-create.sh passes it, since a fresh container has nothing to lose.

Fails fast instead of late

Postgres reachability used to be a printed warning (⚠️ If you use docker to run postgres, make sure your database is running), and Node was never checked at all — so a wrong Node blew up on npm ci at the very end, after the full DB seed. All cheap validation now runs before the first gem install:

• .env created from .env.example, plus a warning listing keys that .env.example defines and .env lacks (previously these silently drifted).
• Exact Ruby version comparison. The old check was current_ruby_version.include?(expected), so ruby 4.0.61 would have passed.
• Node presence and major version against engines.node.
• A real postgres connection attempt with the .env values, via psql (validates the user too) falling back to pg_isready, or skipped with a note if neither is installed.

Errors name the variable to change:

Could not connect to postgres.

Checked host=localhost port=5499 user=postgres, 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.

This replaces the README's "bin/setup fails with a credentials error" troubleshooting entry.

Smaller things

• Drops gem install foreman — bin/dev already installs foreman on demand.
• Drops bin/rails restart, which restarts nothing on a fresh clone.
• Adds --skip-assets and --help.
• $stdout.sync = true, so progress and error output stay in order when piped to a log rather than a terminal (the Codespaces post-create case).
• Ends with the actual next steps — bin/dev, the URL, and the seed login — at the moment you need them.

Verification

Preflight paths exercised individually against a sandboxed app root, all with the real code from bin/setup:

Case	Result
Happy path	passes, exit=0
Wrong Ruby version	aborts, exit=1, prints rbenv/rvm/asdf commands
Postgres down (bad port)	aborts, exit=1, names host/port/user
Bad POSTGRES_USER	aborts, exit=1
Key missing from .env	warns, names the key, continues
Node absent from PATH	aborts, exit=1, points at nvm install
Node major mismatch (v20)	aborts, exit=1
Neither psql nor pg_isready	warns that the DB check was skipped, continues

Full bin/setup run end to end in a clean worktree with no .env (so it exercised first-time setup): green, exit=0, through db:prepare, db:test:prepare, after_party:run, npm ci and both asset builds. standardrb bin/setup is clean.

One caveat on the wrong-password case: my local postgres uses trust auth, so a bad password connects anyway and the check passes. The check does catch a bad user, a bad host and a bad port. Credential validation is therefore only meaningful where postgres actually requires a password — worth knowing, though it is strictly better than the previous warning-only behavior.

A second back-to-back run failed on db:test:prepare with PG::ObjectInUse — a stale rspec process on my machine was holding casa_test. Pre-existing and unrelated; the original script fails identically there.

Docs updated

README.md (troubleshooting entry replaced, flags table added), .devcontainer/post-create.sh (--reset), CLAUDE.md command table.

Not in this PR

bin/setup and bin/update remain near-duplicates with their own divergent system! semantics. Extracting a shared helper would stop them drifting again, but it touches a second entrypoint, so I left it out to keep this diff reviewable.

🤖 Generated with Claude Code

---

Added: shellcheck CI job

The repo has 24 shell scripts and no shell linter, so a new Shell lint workflow runs shellcheck over them.

Scripts are selected by shebang, not by path — most have no extension (bin/dev, docker/*), and bin/ also holds Ruby scripts, so neither a glob nor a directory list gets the set right. Verified on the runner: 24 scripts found, job green in 10s, and no Ruby script picked up.

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. A comment in the workflow says to lower the threshold as they get cleaned up.

Findings fixed to make it green

Two of these were live bugs, not style nits:

bin/git_hooks/lint — the --unpushed path was broken on macOS.

git diff --name-only HEAD~$(git cherry -v main ${current_branch} | wc -l) HEAD

wc -l pads its output with spaces on BSD/macOS, and the substitution was unquoted, so word splitting handed git HEAD~ and 1 as two arguments:

$ git diff --name-only HEAD~$(git cherry -v main "${current_branch}" | wc -l) HEAD
fatal: ambiguous argument '1': unknown revision or path not in the working tree.

Now strips the padding into a variable and quotes it; the rewritten form returns the expected file list. (Linux wc -l doesn't pad, which is why this survived.) Also adds || exit 1 to five bare cds (SC2164), so a failed cd can no longer run a linter against the wrong directory.

docker/run — unquoted $@ re-split arguments (SC2068), so docker/run rspec -e "some example" lost its quoting. Now "$@".

bin/git_hooks/update-dependencies (SC2046, SC1083) — replaced the two-branch [ $(git diff ... | wc -l) -gt 0 ] test with a single git diff --quiet over both pathspecs. Verified equivalent against a purpose-built repo across lockfile-only, package.json-only, and unrelated changes:

reflog range	old expression	new expression
lockfile change	CHANGED	CHANGED
package.json change	CHANGED	CHANGED
unrelated change	unchanged	unchanged

.devcontainer/post-create.sh — added a shebang (SC2148); it had none.

Gate verification

Beyond the green run, I confirmed the job actually fails when it should: adding a new extensionless script with an SC2164 defect was discovered and failed the step (exit 1); fixing the defect returned it to exit 0.

yamllint on the new workflow produces the same two warnings (truthy on on:, comment spacing on the pinned action SHA) as every existing workflow, and no errors.

Unrelated CI failure

ruby_lint is red on this PR, but it is red on main too — the last two main runs failed with Style/RedundantStructKeywordInit in app/services/supervisor_dashboard.rb and app/services/volunteer_dashboard.rb, from a standard/rubocop version bump. Neither file is touched here. Worth its own one-line PR.

[Copilot]

🟡 Changes recommended

bin/setup currently reads DB connection settings only from .env (not existing process ENV), which will break devcontainer setups that supply DATABASE_HOST/POSTGRES_* via containerEnv.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves the first-run developer experience by making bin/setup validate prerequisites early, handle expected failures without raising, and avoid destructive database resets unless explicitly requested.

Changes:

• Reworks bin/setup to add preflight checks (Ruby/Node/Postgres), clearer abort messages, and non-destructive db:prepare by default with an explicit --reset.
• Updates docs to reflect the new setup behavior and flags (--reset, --skip-assets, --help).
• Updates devcontainer post-create to use bin/setup --reset for clean container provisioning.

File summaries

File	Description
README.md	Updates troubleshooting guidance and documents new bin/setup flags.
CLAUDE.md	Updates the one-time setup command description to mention re-run safety and flags.
bin/setup	Adds preflight validation, clearer control flow around dependency install, and safer DB handling with --reset.
.devcontainer/post-create.sh	Runs bin/setup --reset in fresh containers.

Review details

• Files reviewed: 3/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.