fix: close the final review of the shell and comment cleanup

CONTRIBUTING.md

@@ -5,7 +5,7 @@
 ```sh
 mise install            # every tool this repository uses, pinned in mise.toml
 mise exec -- lefthook install   # shellcheck, gitleaks, commit-message check
-mise run lint           # shellcheck over scaffold, lib/, scripts/, common/
+mise run lint           # shellcheck + shfmt over every tracked shell file
 mise run test-unit      # the offline suites
 ```
 
@@ -22,7 +22,7 @@ anything else, so it needs no wrapper — from the clone, a symlink, or `PATH`.
 
 | Task | What it runs |
 | --- | --- |
-| `lint` | shellcheck over every shell file the repository tracks |
+| `lint` | shellcheck + shfmt (`-i 2 -ci`) over every shell file the repository tracks |
 | `test-unit` | the suites that never invoke an adapter's generator — offline and quick |
 | `test-integration` | the suites that generate a real project as a fixture |
 | `test` | every suite, including the per-adapter smoke tests |

adapters/laravel-api/.env.example

@@ -4,5 +4,6 @@ APP_KEY=
 APP_DEBUG=false
 APP_URL=http://localhost:8000
 # the database variables are written by the selected service's driver
-# storage/logs is unreadable in a container, and the readiness probe logs there
+# stderr, because nothing reads storage/logs in a container, where the
+# readiness probe's failures would vanish
 LOG_CHANNEL=stderr

adapters/laravel-inertia/.env.example

@@ -5,5 +5,6 @@ APP_DEBUG=false
 APP_URL=http://localhost:8000
 # the database variables are written by the selected service's driver
 VITE_APP_NAME="${APP_NAME}"
-# storage/logs is unreadable in a container, and the readiness probe logs there
+# stderr, because nothing reads storage/logs in a container, where the
+# readiness probe's failures would vanish
 LOG_CHANNEL=stderr

common/docs/mise.toml

@@ -1,5 +1,5 @@
 [tasks.install]
-# A frozen install that relinks node_modules must not stop to ask (ADR-0017).
+# A frozen install that relinks node_modules must not stop to ask (toolbox ADR-0017).
 env = { npm_config_confirm_modules_purge = "false" }
 run = "pnpm install --frozen-lockfile"
 

common/mise.root.toml

@@ -1,7 +1,7 @@
 monorepo_root = true
 
 # Read by `scaffold add` to wire a new application to the same services.
-# Editing them migrates nothing (ADR-0019).
+# Editing them migrates nothing (toolbox ADR-0019).
 [vars]
 database = "@DATABASE@"
 cache = "@CACHE@"

common/pnpm-workspace.yaml

@@ -2,7 +2,7 @@ packages:
   - apps/*
   - packages/*
   - docs
-# pnpm blocks an unreviewed postinstall build, aborting ci-unit (ADR-0017).
+# pnpm blocks an unreviewed postinstall build, aborting ci-unit (toolbox ADR-0017).
 # esbuild fetches the binary vite runs; the others have pure-js fallbacks.
 allowBuilds:
   unrs-resolver: false

docs/runbook/add-an-adapter.md

@@ -77,7 +77,7 @@ still carries a known, open gap.
 `cmd_add` relaxes `confirmModulesPurge` in `pnpm-workspace.yaml` then
 strips the line back out by blind text match; if the caller had already
 added that exact line themselves, on purpose, this silently deletes it
-too — known, not fixed, see the comment above `reconcile=` in `scaffold`
+too — known, not fixed, see `restore_pnpm_workspace` in `lib/pnpm.sh`
 and ADR-0017's Consequences):
 
 ```bash

lib/pnpm.sh

@@ -64,7 +64,7 @@ pnpm_install() {
   log="$(mktemp)"
 
   (
-    cd "$dir"
+    cd "$dir" || exit 1
     # This install exists to rewrite the lockfile a generator just produced.
     mise exec -- pnpm install \
       --no-frozen-lockfile \

lib/service.sh

@@ -292,17 +292,14 @@ replace_env_line() {
 apply_service_dockerfile() {
   local -r app="$1"
   local block="$2"
-  local file found=0
+  local file
 
   for file in "${app}/Dockerfile" "${app}/Dockerfile.workspace"; do
     [[ -f "$file" ]] || continue
-    found=1
     grep -q "^${SERVICE_SETUP_ANCHOR}\$" "$file" ||
       die "no @SERVICE_SETUP@ anchor in ${file}"
     splice_service_setup "$file" "$block"
   done
-
-  ((found == 1)) || return 0
 }
 
 # ENVIRON, not -v: -v would consume a backslash in the block as an escape.

mise.toml

@@ -27,7 +27,7 @@ run = "bats --jobs $(nproc) --parallel-binary-name rush tests/"
 description = "the suites with no adapter generator anywhere in setup — genuinely offline"
 # Listed, not globbed: new-project.bats generates no adapter; provenance.bats
 # needs an upstream clone (runs in provenance.yml).
-run = "bats --jobs $(nproc) --parallel-binary-name rush tests/add-app-errors.bats tests/cli.bats tests/contract.bats tests/documentation.bats tests/install.bats tests/new-project.bats tests/publish.bats tests/service.bats tests/update.bats tests/wizard.bats"
+run = "bats --jobs $(nproc) --parallel-binary-name rush tests/add-app-errors.bats tests/cli.bats tests/contract.bats tests/documentation.bats tests/install.bats tests/new-project.bats tests/pnpm.bats tests/publish.bats tests/service.bats tests/update.bats tests/wizard.bats"
 
 [tasks."test-integration"]
 description = "suites that generate a real adapter project as a fixture (not a per-adapter smoke test — see tests/new-*.bats for those)"

services/mongodb/drivers/flask.sh

@@ -5,7 +5,7 @@
 # splice_flask_probe, then overrides service_driver_apply,
 # service_driver_compose_env and service_driver_compose_migrate; no FLASK_*
 # variables are set.
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/flask.sh
 . "${SCAFFOLD_ROOT}/services/shared/flask.sh"
 
 service_driver_apply() {

services/mongodb/drivers/laravel.sh

@@ -54,8 +54,9 @@ service_driver_dockerfile() {
   # Pinned to 1.21.0, matching platform.ext-mongodb above: mongodb/mongodb's
   # BSONArray/BSONDocument declare bsonSerialize() against the 1.x signature and
   # the 2.x extension changed it, so loading those classes is a PHP fatal error,
-  # not an exception this project's try/catch can see — a 500 on /health/ready
-  # before this pin.
+  # not an exception this project's try/catch can see — the readiness probe
+  # returns a silent 500 on /health/ready if the extension drifts off this
+  # pin.
   # shellcheck disable=SC2016,SC1003 # literal Dockerfile RUN text: $PHPIZE_DEPS and the
   # trailing backslashes are line continuations in the generated file, not shell escapes
   printf '%s\n' \

services/mongodb/drivers/nest.sh

@@ -9,5 +9,5 @@ PRISMA_PROVIDER="mongodb"
 PRISMA_URL="mongodb://app:app@localhost:27017/app?authSource=admin&directConnection=true"
 # shellcheck disable=SC2016 # literal ${...} written into compose.yaml, not expanded here
 PRISMA_COMPOSE_URL='mongodb://${DB_USERNAME:-app}:${DB_PASSWORD}@database:27017/${DB_DATABASE:-app}?authSource=admin&directConnection=true'
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/nest.sh
 . "${SCAFFOLD_ROOT}/services/shared/nest.sh"

services/mysql/drivers/flask.sh

@@ -7,5 +7,5 @@ FLASK_PACKAGE="PyMySQL"
 FLASK_PORT="3306"
 # shellcheck disable=SC2016 # literal ${...} written into compose.yaml, not expanded here
 FLASK_COMPOSE_URL='mysql+pymysql://${DB_USERNAME:-app}:${DB_PASSWORD}@database:3306/${DB_DATABASE:-app}'
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/flask.sh
 . "${SCAFFOLD_ROOT}/services/shared/flask.sh"

services/mysql/drivers/laravel.sh

@@ -9,5 +9,5 @@ LARAVEL_PACKAGE=""
 LARAVEL_SETUP="RUN docker-php-ext-install pdo_mysql"
 LARAVEL_COMPOSE_ENV="DB_HOST: database
 DB_PORT: 3306"
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/laravel.sh
 . "${SCAFFOLD_ROOT}/services/shared/laravel.sh"

services/mysql/drivers/nest.sh

@@ -5,5 +5,5 @@ PRISMA_PROVIDER="mysql"
 PRISMA_URL="mysql://app:app@localhost:3306/app"
 # shellcheck disable=SC2016 # literal ${...} written into compose.yaml, not expanded here
 PRISMA_COMPOSE_URL='mysql://${DB_USERNAME:-app}:${DB_PASSWORD}@database:3306/${DB_DATABASE:-app}'
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/nest.sh
 . "${SCAFFOLD_ROOT}/services/shared/nest.sh"

services/postgres/drivers/flask.sh

@@ -8,5 +8,5 @@ FLASK_PACKAGE="psycopg[binary]"
 FLASK_PORT="5432"
 # shellcheck disable=SC2016 # literal ${...} written into compose.yaml, not expanded here
 FLASK_COMPOSE_URL='postgresql+psycopg://${DB_USERNAME:-app}:${DB_PASSWORD}@database:5432/${DB_DATABASE:-app}'
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/flask.sh
 . "${SCAFFOLD_ROOT}/services/shared/flask.sh"

services/postgres/drivers/laravel.sh

@@ -8,5 +8,5 @@ LARAVEL_SETUP="RUN apk add --no-cache postgresql-dev \\
  && docker-php-ext-install pdo_pgsql"
 LARAVEL_COMPOSE_ENV="DB_HOST: database
 DB_PORT: 5432"
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/laravel.sh
 . "${SCAFFOLD_ROOT}/services/shared/laravel.sh"

services/postgres/drivers/nest.sh

@@ -5,5 +5,5 @@ PRISMA_PROVIDER="postgresql"
 PRISMA_URL="postgresql://app:app@localhost:5432/app"
 # shellcheck disable=SC2016 # literal ${...} written into compose.yaml, not expanded here
 PRISMA_COMPOSE_URL='postgresql://${DB_USERNAME:-app}:${DB_PASSWORD}@database:5432/${DB_DATABASE:-app}'
-# shellcheck source=/dev/null
+# shellcheck source=services/shared/nest.sh
 . "${SCAFFOLD_ROOT}/services/shared/nest.sh"

services/shared/nest.sh

@@ -13,7 +13,7 @@
 service_driver_apply() {
   # Before the installs, not after: prisma, its engines and its client all place
   # the query engine binary through an install-time script with no pure-js
-  # fallback, and unset the first `pnpm add` below is refused with
+  # fallback, otherwise the first `pnpm add` below is refused with
   # ERR_PNPM_IGNORED_BUILDS wherever CI=true leaves pnpm no prompt.
   # SCAFFOLD_PROJECT_ROOT, not a fixed `../..`: cmd_add's app directory is
   # caller-chosen.
@@ -64,9 +64,9 @@ EOF
 # in eslint's recommended set. A --db none project keeps the throw.
 #
 # The client is a field on HealthController, not a local inside ready(): a
-# controller is a Nest singleton, and a PrismaClient built per request and
-# never closed leaks one connection per poll — measured exhausting Postgres's
-# max_connections inside an hour at a 10s probe interval.
+# controller is a Nest singleton, so a PrismaClient built per request and
+# never closed leaks one connection per poll toward Postgres's
+# max_connections.
 splice_nest_probe() {
   local method field preamble probe
   # shellcheck disable=SC2016 # literal TypeScript spliced into the generated controller
@@ -135,7 +135,7 @@ service_driver_compose_env() {
 # one of two locations depending on which Dockerfile shape wins, a decision made
 # after this driver runs — so the command tries both and `cd`s into whichever
 # matched, since prisma resolves `./prisma/schema.prisma` from its own working
-# directory (measured: `Could not find Prisma Schema` before this `cd`).
+# directory, and fails with `Could not find Prisma Schema` otherwise.
 service_driver_compose_migrate() {
   local args
   case "$PRISMA_PROVIDER" in

tests/new-laravel-api.bats

@@ -51,11 +51,9 @@ teardown() {
 
 @test "a mixed-language project keeps the supply-chain policy" {
   scaffold new "$PROJECT" --api laravel-api --web nextjs
-  # allowBuilds is ADR-0017's policy and applies to any typescript app, but it
-  # lived in the root pnpm-workspace.yaml, which this branch used to delete
-  # outright — so one PHP app in the project removed the policy protecting the
-  # TypeScript one. The packages: list does go, since there is no shared
-  # workspace without a fully TypeScript project; the settings stay.
+  # ADR-0017's allowBuilds lives in the root pnpm-workspace.yaml, so a mixed-
+  # language project keeps the file — deleting it outright would also drop
+  # the TypeScript app's build policy.
   [ -f "${PROJECT}/pnpm-workspace.yaml" ]
   run yq -r '.allowBuilds | keys | .[]' "${PROJECT}/pnpm-workspace.yaml"
   assert_ok
@@ -73,9 +71,9 @@ teardown() {
 
 @test "the adapter's docker directory reaches the generated app" {
   scaffold new "$PROJECT" --api laravel-api
-  # apply_adapter copies a docker/ subtree separately from the flat files, and
-  # nothing asserted it: deleting that line left every test green while the
-  # Dockerfile went on COPYing a file that was no longer there.
+  # apply_adapter copies a docker/ subtree separately from the flat files;
+  # this asserts it independently, so a Dockerfile that COPYs a file no
+  # longer shipped fails here instead of passing silently.
   [ -f "${PROJECT}/apps/api/docker/opcache.ini" ]
   run grep -c 'docker/opcache.ini' "${PROJECT}/apps/api/Dockerfile"
   [ "$output" != "0" ]
@@ -101,10 +99,8 @@ teardown() {
   cd "$PROJECT"
 
   # commitlint backs the commit-msg hook and installs from the project root,
-  # not from an app dir — the one place record_release_age_exceptions used to
-  # skip in this branch, so a violation among commitlint's own dependencies
-  # (too fresh at generation time) surfaced only here, minutes later, on the
-  # first commit.
+  # not from an app dir, so record_release_age_exceptions must cover the root
+  # too or a too-fresh commitlint dependency fails the first commit.
   run mise exec -- pnpm exec commitlint --version
   assert_ok
 }
@@ -128,8 +124,8 @@ teardown() {
 @test "a php-only project still ships the build policy" {
   scaffold new "$PROJECT" --api laravel-api
   # The root package.json is node tooling — commitlint backs the commit-msg
-  # hook — so ADR-0017's allowBuilds applies even with no TypeScript app. This
-  # branch used to delete the file that carries it outright.
+  # hook — so ADR-0017's allowBuilds applies even to a php-only project;
+  # pnpm-workspace.yaml must not be deleted for lack of a TypeScript app.
   [ -f "${PROJECT}/pnpm-workspace.yaml" ]
   run yq -r '.allowBuilds | keys | .[]' "${PROJECT}/pnpm-workspace.yaml"
   assert_ok

tests/new-project.bats

@@ -37,9 +37,8 @@ teardown() {
 
 @test "a relative target is created where the command was run" {
   # scaffold loads its own pinned toolchain through `mise env -C`, which
-  # prints the environment without moving. `mise exec -C` — what the README
-  # used to route every invocation through — moves as well, and this project
-  # then landed inside the toolbox rather than in the caller's directory.
+  # prints the environment without moving — `mise exec -C` moves as well and
+  # would generate the project inside the toolbox instead of here.
   cd "$WORKDIR"
   run scaffold new demo-relative
   assert_ok
@@ -48,9 +47,8 @@ teardown() {
 }
 
 @test "new reports its steps instead of a package manager's output" {
-  # It used to hand the terminal several minutes of progress bars, through
-  # which the one line that mattered — which application is being generated —
-  # never appeared at all.
+  # Hides the package manager's own progress output, so the one line that
+  # matters — which application is being generated — is not buried in it.
   run scaffold new "$PROJECT"
   assert_ok
   [[ "$output" == *"→ "* ]] || {
@@ -164,12 +162,10 @@ teardown() {
 @test "new does not trust a parent config it did not create" {
   printf 'monorepo_root = true\n\n[monorepo]\nconfig_roots = [\n  "x",\n]\n' \
     >"${WORKDIR}/mise.toml"
-  # No skip guard. There used to be one, for the true reason that `CI=true`
-  # makes mise trust every config it finds — but its only two outcomes were
-  # "skipped on a runner" and "failed everywhere else", so the property was
-  # never verified anywhere and the leak it guards against shipped. The state
-  # directory is the suite's own now (tests/helpers/setup), and CI is scrubbed
-  # from the run below, so the precondition holds in both environments.
+  # No CI skip guard: `CI=true` makes mise trust every config it finds, so
+  # this must run everywhere. The suite's own state directory
+  # (tests/helpers/setup) and the scrubbed CI below keep the precondition
+  # true in and out of CI.
   run env -u CI -u MISE_YES -u GITHUB_ACTIONS mise trust --show -C "$WORKDIR"
   [[ "$output" == *"${WORKDIR}: untrusted"* ]] || {
     echo "precondition failed; trust --show reported:"

tests/pnpm.bats

@@ -0,0 +1,29 @@
+#!/usr/bin/env bats
+
+setup() {
+  load 'helpers/setup'
+  source "${SCAFFOLD_ROOT}/lib/log.sh"
+  source "${SCAFFOLD_ROOT}/lib/pnpm.sh"
+  WORKDIR="$(mktemp -d)"
+}
+
+teardown() {
+  remove_workdir "$WORKDIR"
+}
+
+@test "pnpm_install fails and never invokes pnpm when the target directory is missing" {
+  local stub_dir="${WORKDIR}/stub"
+  local ran_marker="${WORKDIR}/pnpm-ran"
+  mkdir -p "$stub_dir"
+  cat >"${stub_dir}/pnpm" <<EOF
+#!/usr/bin/env bash
+touch "${ran_marker}"
+exit 0
+EOF
+  chmod +x "${stub_dir}/pnpm"
+
+  PATH="${stub_dir}:${PATH}" run pnpm_install "${WORKDIR}/missing" "a missing target directory"
+
+  [ "$status" -ne 0 ]
+  [ ! -e "$ran_marker" ]
+}

tests/service.bats

@@ -183,10 +183,9 @@ setup() {
 }
 
 @test "the nest probe guard rejects a controller whose fallback throw survived" {
-  # The guard used to check for `return { status: 'ok' };`, which live() already
-  # returns — so a controller with three splices applied and the fourth missed
-  # passed it, and the project shipped a readiness route that was permanently
-  # 503 with nothing said about it.
+  # assert_nest_probe_spliced must fail on the surviving fallback throw
+  # alone: checking only for live()'s own `return { status: 'ok' };` would
+  # pass a controller with the readiness splice missed.
   source "${SCAFFOLD_ROOT}/services/shared/nest.sh"
 
   local file="${BATS_TEST_TMPDIR}/health.controller.ts"
@@ -590,9 +589,8 @@ EOF
   run yq -e '.services.database != null and .services.cache != null' \
     "${project}/compose.yaml"
   assert_ok
-  # assemble_compose no longer touches an application service — a project has
-  # one per application now, and add_app_service is what makes each of them
-  # wait on the services it was generated against (ADR-0022).
+  # assemble_compose wires only the shared database and cache services;
+  # add_app_service is what makes each application wait on them (ADR-0022).
   # Compared as a joined string: yq's `==` on two sequences returns a
   # sequence of per-element results, not one boolean, so `-e` reads it as no
   # match and the test fails whatever the keys are.
@@ -793,7 +791,7 @@ _password_literal_report() {
   block="$(
     . "${SCAFFOLD_ROOT}/lib/service.sh"
     SERVICE_DIR="$(dirname "$(dirname "$driver")")"
-    # shellcheck source=/dev/null
+    # shellcheck source=/dev/null # path varies by driver
     . "$driver"
     service_driver_compose_env
   )"
@@ -898,7 +896,7 @@ _password_literal_report() {
   for service in mysql postgres mongodb; do
     block="$(
       . "${SCAFFOLD_ROOT}/lib/service.sh"
-      # shellcheck source=/dev/null
+      # shellcheck source=/dev/null # path varies by service
       . "${SCAFFOLD_ROOT}/services/${service}/drivers/laravel.sh"
       service_driver_compose_env
     )"
@@ -916,7 +914,7 @@ _password_literal_report() {
   for service in mysql postgres mongodb; do
     block="$(
       . "${SCAFFOLD_ROOT}/lib/service.sh"
-      # shellcheck source=/dev/null
+      # shellcheck source=/dev/null # path varies by service
       . "${SCAFFOLD_ROOT}/services/${service}/drivers/nest.sh"
       service_driver_compose_env
     )"