From dbe333d0d97485dd1c33c40fe3017860a4f85ecb Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 14 Oct 2026 10:18:32 -0400 Subject: [PATCH 1/2] Stop the required validate-profile check from passing by being skipped The code part of this bug merged in #48: main-branch-ruleset.json requires a pull request and the validate-profile check, and a daily job fails while main is unprotected. Main is still unprotected today; the 2026-10-14 scheduled run failed and filed issue #49. The ruleset had a gap that would open once it is applied. validate-profile had a job-level condition and needed reject-lifecycle-app, so on a pull request the lifecycle App opens, the job was skipped. GitHub counts a skipped job as a passing required check, so that pull request would show validate-profile as passed although validation never ran. validate-profile now has no job-level condition or needs. Its first step refuses the lifecycle App and fails, before checkout. The check is red, not skipped, on App-triggered runs. A person closes and reopens the pull request to run validation as themselves. The new test reads the required checks from the committed ruleset and fails if the job behind any of them can be skipped. Ledger: B247 Signed-off-by: abrichr Co-Authored-By: Claude Opus 5.5 --- .github/workflows/profile-consistency.yml | 14 ++++++--- tests/test_branch_protection.py | 37 +++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/.github/workflows/profile-consistency.yml b/.github/workflows/profile-consistency.yml index 22f79ce..6d02be6 100644 --- a/.github/workflows/profile-consistency.yml +++ b/.github/workflows/profile-consistency.yml @@ -29,13 +29,19 @@ jobs: test "$TRIGGERING_ACTOR" != 'openadapt-lifecycle[bot]' validate-profile: - needs: reject-lifecycle-app - if: >- - github.actor != 'openadapt-lifecycle[bot]' && - github.triggering_actor != 'openadapt-lifecycle[bot]' + # Main's ruleset requires this check, and GitHub counts a skipped job as a + # passing check. So this job has no job-level condition or needs that + # could skip it. It refuses the lifecycle App with a failing first step. runs-on: ubuntu-latest timeout-minutes: 5 steps: + - name: Reject the lifecycle App + env: + ACTOR: ${{ github.actor }} + TRIGGERING_ACTOR: ${{ github.triggering_actor }} + run: | + test "$ACTOR" != 'openadapt-lifecycle[bot]' + test "$TRIGGERING_ACTOR" != 'openadapt-lifecycle[bot]' - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: fetch-depth: 0 diff --git a/tests/test_branch_protection.py b/tests/test_branch_protection.py index f43bb97..59d5327 100644 --- a/tests/test_branch_protection.py +++ b/tests/test_branch_protection.py @@ -5,6 +5,7 @@ import copy import io import json +import re import sys import unittest from contextlib import redirect_stderr, redirect_stdout @@ -300,6 +301,42 @@ def test_profile_workflow_runs_the_check_on_schedule(self) -> None: self.assertIn(" report-unprotected-branch:\n", content) self.assertIn("needs.check-branch-protection.result == 'failure'", content) + def test_required_check_fails_instead_of_skipping(self) -> None: + # GitHub counts a skipped job as a passing required check. A pull + # request the lifecycle App opens would then show the required check + # as passed although validation never ran. So the job behind each + # required check has no job-level condition or dependency that could + # skip it, and it refuses the lifecycle App with a failing first step. + body = json.loads((ROOT / "main-branch-ruleset.json").read_text()) + contexts = [ + item["context"] + for rule in body["rules"] + if rule["type"] == "required_status_checks" + for item in rule["parameters"]["required_status_checks"] + ] + self.assertEqual(contexts, [check.REQUIRED_CHECK]) + content = (ROOT / ".github/workflows/profile-consistency.yml").read_text() + for context in contexts: + job = content.split(f"\n {context}:\n", 1)[1] + job = re.split(r"\n (?=\S)", job, maxsplit=1)[0] + self.assertNotRegex(job, r"(?m)^ if:", context) + self.assertNotRegex(job, r"(?m)^ needs:", context) + first_step = job.split(" steps:\n - ", 1)[1] + first_step = first_step.split("\n - ", 1)[0] + self.assertNotIn("uses:", first_step, context) + self.assertIn("ACTOR: ${{ github.actor }}", first_step, context) + self.assertIn( + "TRIGGERING_ACTOR: ${{ github.triggering_actor }}", first_step, context + ) + self.assertIn( + "test \"$ACTOR\" != 'openadapt-lifecycle[bot]'", first_step, context + ) + self.assertIn( + "test \"$TRIGGERING_ACTOR\" != 'openadapt-lifecycle[bot]'", + first_step, + context, + ) + if __name__ == "__main__": unittest.main() From ff04ce6fb651450a9612f120ddfc3d12e4683d17 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 14 Oct 2026 10:32:54 -0400 Subject: [PATCH 2/2] Fail the skip test when the refusal step can be skipped or ignored The regression test for B247 checked that validate-profile has no job-level condition or needs, and that its first step refuses the lifecycle App. It still passed in three cases that weaken the refusal. A step-level if can skip the refusal. continue-on-error on the refusal step turns its failure into a success, so the job goes on and the check passes. The test now fails on both, and on continue-on-error at the job level as well. A mutation run on the workflow shows the earlier test missed these three cases. The new test catches them and the five cases the earlier test already caught. Ledger: B247 Signed-off-by: abrichr Co-Authored-By: Claude Opus 5.5 --- tests/test_branch_protection.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/test_branch_protection.py b/tests/test_branch_protection.py index 59d5327..6a127dd 100644 --- a/tests/test_branch_protection.py +++ b/tests/test_branch_protection.py @@ -321,9 +321,16 @@ def test_required_check_fails_instead_of_skipping(self) -> None: job = re.split(r"\n (?=\S)", job, maxsplit=1)[0] self.assertNotRegex(job, r"(?m)^ if:", context) self.assertNotRegex(job, r"(?m)^ needs:", context) + # Nothing may let the job run on past a failed refusal. + self.assertNotRegex(job, r"(?m)^ continue-on-error:", context) first_step = job.split(" steps:\n - ", 1)[1] first_step = first_step.split("\n - ", 1)[0] self.assertNotIn("uses:", first_step, context) + # A condition could skip the refusal, and continue-on-error would + # let a failed refusal count as a success. + self.assertNotRegex( + first_step, r"(?m)^(?: {8})?(?:if|continue-on-error):", context + ) self.assertIn("ACTOR: ${{ github.actor }}", first_step, context) self.assertIn( "TRIGGERING_ACTOR: ${{ github.triggering_actor }}", first_step, context