Skip to content

ci(pr): make InspectCode solution discovery robust under errexit - #274

Merged
Chris-Wolfgang merged 1 commit into
mainfrom
fix/inspectcode-slnx-glob-errexit
Jul 18, 2026
Merged

ci(pr): make InspectCode solution discovery robust under errexit#274
Chris-Wolfgang merged 1 commit into
mainfrom
fix/inspectcode-slnx-glob-errexit

Conversation

@Chris-Wolfgang

@Chris-Wolfgang Chris-Wolfgang commented Jul 18, 2026

Copy link
Copy Markdown
Owner

The blocker

#267 merged the InspectCode job into main at 16:04. Because pr.yaml runs via pull_request_target, every open PR now runs InspectCode from main — and it fails at the "Run InspectCode" step with exit code 2, before jb even starts (the build step passes). #245 and every other open PR are blocked on this.

Root cause

GitHub's shell: bash runs with -e -o pipefail. The step picked a solution by parsing ls:

SLN=$(ls *.slnx 2>/dev/null | head -n1)

This repo has ETL-Abstractions.sln but no .slnx, so ls *.slnx exits 2 → pipefail propagates it → set -e aborts the whole step before the .sln fallback runs. This breaks InspectCode on every repo that has only a .sln (i.e. essentially all of them).

It slipped through because InspectCode needs: detect-projects, and on #267's own PR the protected-file guard failed that job, so InspectCode was skipped and never actually executed before #267 was bypass-merged.

Fix

Rather than band-aid the ls pipeline with || true (which would also mask genuine failures), replace the ls-parsing with nullglob + array expansion:

shopt -s nullglob
solutions=(*.slnx *.sln)
if [ ${#solutions[@]} -eq 0 ]; then
  echo "::error::No .slnx or .sln found at repo root — InspectCode needs one to run."
  exit 1
fi
SLN="${solutions[0]}"

A non-matching glob expands to nothing instead of failing, so discovery is correct under errexit/pipefail by construction — no ls exit-2 to trip on. Listing *.slnx before *.sln keeps .slnx preferred. Also puts ~/.dotnet/tools on PATH so the jb global tool resolves.

Verified in a local bash -e -o pipefail harness across all four cases: only-.sln, only-.slnx, both (.slnx wins), and neither (fails loudly, exit 1).

Merge note

This touches .github/workflows/pr.yaml (a protected file), so Detect .NET Projects will fail by design — it's the only expected failure. Needs an admin-bypass merge. Once merged, I'll re-run InspectCode on #245 (and the other open PRs) to confirm green.

The same bug exists in the canonical repo-template; I'll fold this fix into the open feed-back PR #439.

Copilot AI review requested due to automatic review settings July 18, 2026 16:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The Run InspectCode step selected a solution by parsing ls:

    SLN=$(ls *.slnx 2>/dev/null | head -n1)

Under GitHub's `shell: bash` (-e -o pipefail), when no .slnx exists `ls`
exits 2, pipefail propagates it, and errexit aborts the whole step (exit 2)
before the .sln fallback runs. Every repo that has only a .sln — i.e.
essentially all of them — fails InspectCode here. It slipped through because
InspectCode needs detect-projects, which failed the protected-file guard on
the job's own introducing PR, so the step never executed before merge.

Replace the ls-parsing with nullglob + array expansion: a non-matching glob
expands to nothing rather than failing, so discovery is correct under
errexit/pipefail by construction — no ls exit-2 to trip on, and no blanket
`|| true` that would also mask a genuine failure. Listing *.slnx before
*.sln keeps .slnx preferred. Also put ~/.dotnet/tools on PATH so `jb`
resolves regardless of runner defaults.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Chris-Wolfgang
Chris-Wolfgang force-pushed the fix/inspectcode-slnx-glob-errexit branch from 8b94d21 to 9d287af Compare July 18, 2026 16:40
@Chris-Wolfgang Chris-Wolfgang changed the title ci(pr): fix InspectCode step aborting on repos with only a .sln ci(pr): make InspectCode solution discovery robust under errexit Jul 18, 2026
@Chris-Wolfgang
Chris-Wolfgang merged commit bfa8d45 into main Jul 18, 2026
10 of 11 checks passed
@Chris-Wolfgang
Chris-Wolfgang deleted the fix/inspectcode-slnx-glob-errexit branch July 18, 2026 16:47
Chris-Wolfgang added a commit to Chris-Wolfgang/repo-template that referenced this pull request Jul 18, 2026
Same fix as Chris-Wolfgang/ETL-Abstractions#274. The Run InspectCode step
parsed `ls *.slnx` to pick a solution; under GitHub's `shell: bash`
(-e -o pipefail) a no-match `ls` exits 2, pipefail propagates it, and errexit
aborts the step before the .sln fallback runs — so any repo that has only a
.sln fails InspectCode. Replace ls-parsing with nullglob + array expansion
(a non-matching glob expands to nothing, correct under errexit by
construction) and put ~/.dotnet/tools on PATH so `jb` resolves.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants