skip exits with 1 for the buildkite pr build analyser #1946

Activity

1. github-actions commented on Aug 28, 2026

   

   github-actionsbot

   on Aug 28, 2026 – with GitHub Actions

   Contributor

   More actions

   tl;dr: the workflow currently treats skip as a hard failure (exit 1), so your issue is valid — the fix should make skip paths exit successfully and gate downstream analysis on an explicit signal.

   Recommendation

   Update .github/workflows/gh-aw-estc-pr-buildkite-detective.md so skip conditions do not fail the step/job. Concretely, change the skip() helper to sys.exit(0) and add a small explicit output/signal (for example should_analyze=true|false) that downstream steps use in if: guards.

   This preserves current behavior for real errors while making expected no-op conditions (no PR, non-failed build, no failed script jobs) cleanly skip analysis.

   Findings

   • In the source workflow, skip is explicitly implemented as failure:
     • .github/workflows/gh-aw-estc-pr-buildkite-detective.md:139-143
     • def skip(reason): ... sys.exit(1)
   • Skip is invoked for expected/non-error paths:
     • .github/workflows/gh-aw-estc-pr-buildkite-detective.md:145 (no PR)
     • .github/workflows/gh-aw-estc-pr-buildkite-detective.md:148 (build state not failed/failing)
     • .github/workflows/gh-aw-estc-pr-buildkite-detective.md:172 (no failed script jobs)
   • The generated lock file mirrors this behavior (so this is not a compile mismatch):
     • .github/workflows/gh-aw-estc-pr-buildkite-detective.lock.yml:573
   • Related issues I checked (#1098, #647) are about comment spam/reuse and do not address skip exit-code semantics.

   Verification

   I validated the behavior by inspecting the source and generated workflow files and by simulating the skip() control path: it returns exit code 1 today.

   Detailed Action Plan

   1. In .github/workflows/gh-aw-estc-pr-buildkite-detective.md, update the Python helper:
      • At skip(reason), replace sys.exit(1) with sys.exit(0).
   2. Emit an explicit step output for downstream control (e.g., should_analyze=false on skip, true when failures are found).
   3. Guard subsequent analysis/comment steps with if: ... && steps.<resolve-step>.outputs.should_analyze == 'true' so skip short-circuits the rest.
   4. Re-run make compile so .lock.yml regenerates from the source.
   5. Add/update a focused test/fixture (if present for workflow rendering/behavior) to assert skip scenarios are non-failing.

   Related Items

   Type	Link	Relevance
   Issue	#1946	Current report: skip should not fail run
   Issue	#1098	Same workflow family; comment spam concern (different problem)
   Issue	#647	Same workflow family; comment reuse concern (different problem)
   File	.github/workflows/gh-aw-estc-pr-buildkite-detective.md:139-173	Source of skip logic and call sites
   File	.github/workflows/gh-aw-estc-pr-buildkite-detective.lock.yml:573	Generated workflow confirms same skip semantics

   ---

   What is this? | From workflow: Trigger Issue Triage

   Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
2. v1v

   assigned

   Copilot

   and

   v1v

   on Aug 28, 2026
3. Copilot

   linked a pull request that will close this issueFix: Buildkite detective skip conditions exit cleanly instead of failing #1947on Aug 28, 2026
4. Copilot

   added a commit that references this issue on Aug 28, 2026

   Fix: Change skip exit code from 1 to 0 for buildkite detective

   ...

   This issue will close once commit 912e69f is merged into the 'main' branch.

   912e69f