fix(fork): install shellcheck before linting the launch gate

[NoahHendrickson]

The first dry_run dispatch failed with shellcheck: command not found at the gate step — current macos-latest images no longer preinstall shellcheck — after spending the full build. One line: install the bottle when absent (takes seconds), keeping the lint ahead of the gate.

🤖 Generated with Claude Code

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[cursor]

Thermo-nuclear code quality review: no major structural issues found.

This is the minimal correct fix for the dry-run failure (shellcheck: command not found on current macos-latest). The conditional brew install is direct and keeps lint ahead of the gate without new branching debt, abstractions, or workflow sprawl. Approval bar met.

  

_{Sent by Cursor Automation: Thermo-nuclear PR review}

[NoahHendrickson]

Code review — fix(fork): install shellcheck before linting the launch gate

Verdict: request changes. The diagnosis is right, but the fix doesn't clear the failure. The next dry_run still fails at the same step, still after paying for the full build — just with a different message.

---

🔴 Blocking — the gate script is not shellcheck-clean, so the lint fails as soon as it can actually run

shellcheck defaults to --severity=style, and info-level findings exit non-zero. .github/scripts/launch-isolation-check.sh has one:

$ shellcheck .github/scripts/launch-isolation-check.sh

In .github/scripts/launch-isolation-check.sh line 110:
  ls -t "$HOME/Library/Logs/DiagnosticReports" 2>/dev/null | head -5 >&2 || true
  ^-- SC2012 (info): Use find instead of ls to better handle non-alphanumeric filenames.

exit=1

Reproduced against shellcheck 0.11.0 (what brew install shellcheck fetches today) and 0.9.0 — same result. There is no .shellcheckrc in the repo and no # shellcheck disable directive in the script, so nothing suppresses it.

The first dry run died at command -v shellcheck before the lint ever executed, which is why this was invisible. This PR removes that shield and exposes the real failure underneath. And because the "Launch isolation check" step carries no if:, this blocks real publishes too, not just dry runs.

Minimal unblock — a targeted directive, in the file's own commented idiom (the ls -t is deliberate; newest-first ordering is the whole point, and the filenames are runner-generated):

   echo "--- newest crash reports (real home) ---" >&2
   # The macOS crash reporter writes to the real user home, not $HOME.
+  # shellcheck disable=SC2012 -- newest-first ordering is the point, and these
+  # filenames are crash-reporter-generated, not hostile.
   ls -t "$HOME/Library/Logs/DiagnosticReports" 2>/dev/null | head -5 >&2 || true

Please run the lint locally before the next dispatch. shellcheck --severity=warning on this script is already clean at exit 0 — that's the other lever, discussed below.

---

🟠 Structural — this lint is in the wrong job, and that's the actual bug class

The thing being linted is a static file already committed to the repo. Nothing about the release build changes it. Yet the lint sits behind a 20–40 minute vp run dist:desktop:artifact, on the one runner tier where the tool isn't preinstalled, in a job that costs a version number to iterate on. That ordering is what turned a two-character oversight into a burned build — and this PR keeps the ordering and just patches the symptom.

CI already has the right home for it. .github/workflows/ci.yml has a release_smoke job (ubuntu-latest, runs on every pull_request) whose stated purpose is "Exercise release-only workflow steps":

      - name: Lint release scripts
        run: shellcheck .github/scripts/launch-isolation-check.sh

Ubuntu runner images ship shellcheck, so that's a zero-install step. (If that ever stops being true, sudo apt-get install -y shellcheck there costs seconds and — crucially — isn't standing behind a build.) Then fork-release.yml drops the install and the lint entirely and the step becomes what it says on the tin:

        run: bash .github/scripts/launch-isolation-check.sh release

Net effect: a lint defect is caught on the PR that introduces it, in seconds, at zero risk to a release — instead of on a dispatch, after a build, at the cost of a version number. Same coverage, failure moved ~40 minutes earlier and one job to the left.

---

🟡 An unpinned tool now has veto power over releases

brew install shellcheck resolves to whatever HEAD-of-formula is on the day of the dispatch. A future shellcheck adding a new default-severity check will break releases with zero repository change, discovered only after a build. That's not hypothetical — it's the mechanism of the SC2012 failure above, one step removed.

Two mitigations, pick one:

1. Bound the severity to the classes worth blocking a release on: shellcheck --severity=warning … (verified clean on this script today). Style/info drift then can't veto a publish.
2. Declare the dependency the way this repo already does. apps/mobile/Brewfile + brew bundle install --file apps/mobile/Brewfile in ci.yml is the established convention for brew-installed lint tooling here; this line invents a second, ad-hoc mechanism alongside it.

---

🟡 The "installs in seconds" claim isn't true as written

brew install on GitHub-hosted runners triggers an implicit brew update first, which routinely adds minutes and is itself a well-known flake source. The comment's justification only holds with auto-update off:

+          export HOMEBREW_NO_AUTO_UPDATE=1
           command -v shellcheck >/dev/null || brew install --quiet shellcheck

Worth naming the tradeoff plainly: this change introduces a new outbound network dependency into the post-build critical path of a release. A Homebrew CDN hiccup now costs a full build — which is precisely the failure shape this PR set out to eliminate. Moving the lint left (above) removes the dependency instead of hardening it.

---

🔵 Nits

• Missing set -euo pipefail. Every other multi-line run: in this file opens with it — lines 66, 99, 133. This step doesn't. Behavior is fine today (shell: bash gives GitHub's bash --noprofile --norc -eo pipefail {0}, so errexit and pipefail hold; -u doesn't), but the file's convention is to be explicit, and a reader shouldn't have to know the runner default to be sure a failing shellcheck fails the step.
• --quiet trims exactly the output you'd want when diagnosing a failed install. Low stakes, but the flag buys nothing here — the noise it suppresses only appears on a path you rarely take.

---

Suggested shape

If the structural change is in scope, do that: move the lint to release_smoke, delete the install and the lint from fork-release.yml, and add the SC2012 directive so the new CI step is green on its first run.

If you'd rather keep this PR minimal and land the structural change separately, then this step needs all three of:

        run: |
          set -euo pipefail
          export HOMEBREW_NO_AUTO_UPDATE=1
          command -v shellcheck >/dev/null || brew install --quiet shellcheck
          shellcheck --severity=warning .github/scripts/launch-isolation-check.sh
          bash .github/scripts/launch-isolation-check.sh release

…plus the SC2012 directive regardless, since --severity=warning is a bound, not a fix.

Do not merge and dispatch as-is — the dry run will fail at the same step for a new reason, at the same cost.

---

_{Reviewed at commit 1f588c1. SC2012 reproduced locally against shellcheck 0.9.0 and 0.11.0; --severity=warning verified clean at exit 0. The Homebrew auto-update and ubuntu-runner-image claims are from general runner behavior and worth a quick confirm on your side.}

---

Generated by Claude Code

[NoahHendrickson]

Response — structural option taken @ d06206e68

Blocker confirmed and fixed. SC2012 reproduced locally against shellcheck 0.11.0 exactly as described; a targeted directive suppresses it and the script now lints clean at default severity — verified before this push, as requested. Two corrections to the review's own suggested diff, both verified against 0.11.0: shellcheck directives cannot wrap across lines (the two-line form in the review fails with SC1072/SC1073), and the -- reason trailing-comment syntax does not exist in any released shellcheck — the rationale lives on its own comment line above the directive.

Structural finding — done in full. The lint moved to ci.yml's release_smoke job (a fenced fork hunk; ubuntu images ship shellcheck, zero install), and the release workflow's gate step is now exactly bash .github/scripts/launch-isolation-check.sh release. The brew install is gone rather than hardened — which also dissolves the unpinned-tool-veto finding and the auto-update/network-dependency finding at the root, as the review predicted. A lint defect now costs a PR check on the PR that introduces it, never a build.

Guard and manifest: the fork guard asserts the gate step runs the script with no if: and that ci.yml carries the lint; .fork/customizations.yaml adds ci.yml to fork-desktop-release's watch list for the new fenced hunk. Fork guards: 50 passing.

The nits (missing set -euo pipefail, --quiet) died with the step they applied to.

🤖 Generated with Claude Code