Make CI checks safe to require on main

.github/workflows/0-ci-build-and-test.yml

@@ -104,8 +104,8 @@ jobs:
   # build dependency.
   docs-render:
     name: Docs - Markdown rendering and in-repo links
-    needs: changes
-    if: ${{ needs.changes.outputs.docs == 'true' }}
+    # Unconditional: it checks the whole tree in seconds, and a required status
+    # check must never be skipped or it reports as pending forever.
     runs-on: ubuntu-latest
     timeout-minutes: 5
     steps:
@@ -205,28 +205,40 @@ jobs:
   unit-tests:
     name: Unit tests
     needs: changes
-    # always() so a detector failure degrades to running this gate rather than
-    # silently skipping it; an empty docs_only is treated as "not docs-only".
-    if: ${{ always() && needs.changes.outputs.docs_only != 'true' }}
+    # These three jobs always run so they can safely be required status checks:
+    # a *skipped* required check reports as pending forever and wedges the PR.
+    # A docs-only change set short-circuits the steps instead, so the check still
+    # reports a conclusion within seconds. An empty docs_only (which is what a
+    # failed `changes` job yields) is treated as "not docs-only", so a detector
+    # failure degrades to running the full gate.
+    if: ${{ always() }}
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
+      - name: Docs-only short-circuit
+        if: needs.changes.outputs.docs_only == 'true'
+        run: echo "Documentation-only change set - nothing to compile or test."
+
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        if: needs.changes.outputs.docs_only != 'true'
 
       - name: Set up JDK 17
+        if: needs.changes.outputs.docs_only != 'true'
         uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.0.0
         with:
           distribution: temurin
           java-version: "17"
 
       - name: Set up Gradle
+        if: needs.changes.outputs.docs_only != 'true'
         uses: gradle/actions/setup-gradle@4c125117fe7c5aed11272ec4213f602f012f89f2 # v5.0.0
 
       - name: Run unit tests
+        if: needs.changes.outputs.docs_only != 'true'
         run: ./gradlew --no-daemon testDebugUnitTest
 
       - name: Upload unit test reports
-        if: always()
+        if: ${{ always() && needs.changes.outputs.docs_only != 'true' }}
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
         with:
           name: hermes-unit-test-reports
@@ -238,26 +250,34 @@ jobs:
   android-lint:
     name: Android Lint
     needs: changes
-    if: ${{ always() && needs.changes.outputs.docs_only != 'true' }}
+    if: ${{ always() }}
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
+      - name: Docs-only short-circuit
+        if: needs.changes.outputs.docs_only == 'true'
+        run: echo "Documentation-only change set - nothing to lint."
+
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        if: needs.changes.outputs.docs_only != 'true'
 
       - name: Set up JDK 17
+        if: needs.changes.outputs.docs_only != 'true'
         uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.0.0
         with:
           distribution: temurin
           java-version: "17"
 
       - name: Set up Gradle
+        if: needs.changes.outputs.docs_only != 'true'
         uses: gradle/actions/setup-gradle@4c125117fe7c5aed11272ec4213f602f012f89f2 # v5.0.0
 
       - name: Run Android Lint
+        if: needs.changes.outputs.docs_only != 'true'
         run: ./gradlew --no-daemon lintDebug
 
       - name: Upload lint report
-        if: always()
+        if: ${{ always() && needs.changes.outputs.docs_only != 'true' }}
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
         with:
           name: hermes-lint-report
@@ -267,19 +287,26 @@ jobs:
   debug-build:
     name: Debug APK build
     needs: changes
-    if: ${{ always() && needs.changes.outputs.docs_only != 'true' }}
+    if: ${{ always() }}
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
+      - name: Docs-only short-circuit
+        if: needs.changes.outputs.docs_only == 'true'
+        run: echo "Documentation-only change set - nothing to build."
+
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        if: needs.changes.outputs.docs_only != 'true'
 
       - name: Set up JDK 17
+        if: needs.changes.outputs.docs_only != 'true'
         uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.0.0
         with:
           distribution: temurin
           java-version: "17"
 
       - name: Prepare Android SDK
+        if: needs.changes.outputs.docs_only != 'true'
         shell: bash
         run: |
           sdkmanager="$ANDROID_HOME/cmdline-tools/latest/bin/sdkmanager"
@@ -289,13 +316,15 @@ jobs:
           set -o pipefail
 
       - name: Set up Gradle
+        if: needs.changes.outputs.docs_only != 'true'
         uses: gradle/actions/setup-gradle@4c125117fe7c5aed11272ec4213f602f012f89f2 # v5.0.0
 
       - name: Assemble debug APK
+        if: needs.changes.outputs.docs_only != 'true'
         run: ./gradlew --no-daemon assembleDebug
 
       - name: Upload debug APK
-        if: success()
+        if: ${{ success() && needs.changes.outputs.docs_only != 'true' }}
         uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7
         with:
           name: hermes-debug-apk

AGENTS.md

@@ -223,7 +223,9 @@ automation changes.
 
 Separate from release publishing, CI uses `.github/workflows/0-ci-build-and-test.yml` to gate pull requests and direct `main` pushes without signing secrets. Each check runs as its own job so a failure names the gate that broke: `docs-render`, `docs-links`, `release-tooling-tests`, `webui-script-syntax`, `webui-script-lint`, `unit-tests`, `android-lint`, and `debug-build`. Keep every job's `timeout-minutes` set; an untimed job can burn the six-hour runner default. Pull requests and direct `main` pushes that change Android source or build inputs also run the complete unfiltered `connectedDebugAndroidTest` suite on Android API 35 and 36. Release builds run the full API 36 suite again, then verify APK and AAB signatures before upload. Keep contributor verification steps aligned with these gates when changing build/test flow.
 
-A documentation-only change set (every changed path ends in `.md`) skips `unit-tests`, `android-lint`, and `debug-build`, which have nothing to compile. The detector in the `changes` job is fail-safe by construction: it always exits 0, always emits `docs_only`, and defaults to `false`, so a missing diff base, an unreadable diff, or a single non-doc path runs the full suite. Those three jobs also use `always()` so a detector failure degrades to running them rather than skipping them. Keep `release-tooling-tests` outside the fast path — it asserts that README release metadata matches Gradle, which is exactly what a docs-only change can break. None of these are required status checks today; if that changes, convert the skipped jobs to short-circuited steps first, because a skipped required check reports as pending forever.
+A documentation-only change set (every changed path ends in `.md`) short-circuits the steps inside `unit-tests`, `android-lint`, and `debug-build`, which have nothing to compile. Those jobs still run and still report a conclusion, because they are required status checks and GitHub reports a *skipped* required check as pending forever. Never convert them back to a job-level `if` that skips them, and keep every step guarded so a docs-only run stays cheap. The detector in the `changes` job is fail-safe by construction: it always exits 0, always emits `docs_only`, and defaults to `false`, so a missing diff base, an unreadable diff, or a single non-doc path runs the full suite; an empty value from a failed detector is treated as "not docs-only". Keep `release-tooling-tests` outside the fast path — it asserts that README release metadata matches Gradle, which is exactly what a docs-only change can break.
+
+The `main` branch requires these checks to pass before merge: `docs-render`, `release-tooling-tests`, `webui-script-syntax`, `webui-script-lint`, `unit-tests`, `android-lint`, and `debug-build`. All seven run unconditionally. `docs-links` and the two `android-ui-smoke` matrix jobs are deliberately not required — the first depends on the network, and the second is path-filtered and therefore skippable. Before making any job required, confirm it can never be skipped.
 
 Docs checks are deliberately minimal and are not a Markdown style linter. `tools/check_markdown.py` blocks only on rendering breaks (an unclosed inline link, or a destination split across a newline) and on relative links or images pointing at a file that does not exist. The `docs-links` job checks external URLs with lychee and stays `continue-on-error` because the network is not a build dependency.
 

tools/tests/test_workflow_contracts.py

@@ -13,6 +13,14 @@ def read_workflow(name: str) -> str:
     return (WORKFLOW_DIR / name).read_text(encoding="utf-8")
 
 
+def ci_job_blocks(workflow: str) -> dict[str, str]:
+    """Map each job id in the CI workflow to its YAML block."""
+    jobs_section = workflow.split("\njobs:\n", 1)[1]
+    names = re.findall(r"^  ([a-z0-9-]+):$", jobs_section, re.MULTILINE)
+    blocks = re.split(r"^  [a-z0-9-]+:$", jobs_section, flags=re.MULTILINE)[1:]
+    return dict(zip(names, blocks))
+
+
 class WorkflowContractTests(unittest.TestCase):
     def test_every_external_action_is_pinned_to_commit(self) -> None:
         failures: list[str] = []
@@ -81,34 +89,66 @@ def test_ci_checks_documentation_when_markdown_changes(self) -> None:
 
     def test_docs_only_change_sets_skip_the_gradle_gates_but_fail_safe(self) -> None:
         workflow = read_workflow("0-ci-build-and-test.yml")
-        jobs_section = workflow.split("\njobs:\n", 1)[1]
-        blocks = dict(
-            zip(
-                re.findall(r"^  ([a-z0-9-]+):$", jobs_section, re.MULTILINE),
-                re.split(r"^  [a-z0-9-]+:$", jobs_section, flags=re.MULTILINE)[1:],
-            )
-        )
+        blocks = ci_job_blocks(workflow)
         for job in ("unit-tests", "android-lint", "debug-build"):
-            self.assertIn(
-                "if: ${{ always() && needs.changes.outputs.docs_only != 'true' }}",
-                blocks[job],
-                msg=f"{job} must skip docs-only runs while defaulting to running",
+            block = blocks[job]
+            header = block.split("\n    steps:\n", 1)[0]
+            directives = "\n".join(
+                line for line in header.splitlines() if not line.strip().startswith("#")
+            )
+            # The job itself must always run. A *skipped* required status check
+            # reports as pending forever, which wedges the pull request.
+            self.assertIn("if: ${{ always() }}", directives, msg=job)
+            self.assertNotIn(
+                "docs_only", directives, msg=f"{job} must not skip at job level"
             )
+            self.assertIn("Docs-only short-circuit", block)
+            # Every step must be guarded, or a docs-only run still pays for it.
+            steps = block.split("\n      - ")[1:]
+            unguarded = [step.splitlines()[0] for step in steps if "docs_only" not in step]
+            self.assertEqual(unguarded, [], msg=f"{job} has unguarded steps")
         # Release tooling tests assert README release metadata, so a docs-only
         # change is exactly when they matter most.
         self.assertNotIn("docs_only", blocks["release-tooling-tests"])
         # The detector must default to running everything.
         self.assertIn('docs_only="false"', blocks["changes"])
         self.assertIn("trap emit EXIT", blocks["changes"])
 
+    def test_required_check_candidates_always_report_a_conclusion(self) -> None:
+        """Jobs intended as required status checks must never be skipped.
+
+        GitHub reports a skipped required check as pending forever, so a job that
+        can be skipped must not be made required. Keep this list aligned with the
+        repository ruleset.
+        """
+        workflow = read_workflow("0-ci-build-and-test.yml")
+        blocks = ci_job_blocks(workflow)
+        required = (
+            "docs-render",
+            "release-tooling-tests",
+            "webui-script-syntax",
+            "webui-script-lint",
+            "unit-tests",
+            "android-lint",
+            "debug-build",
+        )
+        for job in required:
+            block = blocks[job]
+            condition = re.search(r"^    if: (.+)$", block, re.MULTILINE)
+            if condition is None:
+                continue
+            self.assertEqual(
+                condition.group(1).strip(),
+                "${{ always() }}",
+                msg=f"{job} is a required check and must not be conditionally skipped",
+            )
+
     def test_every_ci_job_declares_a_timeout(self) -> None:
         workflow = read_workflow("0-ci-build-and-test.yml")
-        jobs_section = workflow.split("\njobs:\n", 1)[1]
-        jobs = re.findall(r"^  ([a-z0-9-]+):$", jobs_section, re.MULTILINE)
-        self.assertGreater(len(jobs), 1)
-        blocks = re.split(r"^  [a-z0-9-]+:$", jobs_section, flags=re.MULTILINE)[1:]
+        blocks = ci_job_blocks(workflow)
+        self.assertGreater(len(blocks), 1)
         missing = [
-            job for job, block in zip(jobs, blocks) if "timeout-minutes:" not in block
+            job for job, block in blocks.items() if "timeout-minutes:" not in block
         ]
         self.assertEqual(missing, [])