| 1 | --- |
| 2 | title: Release-time consistency tests cause cascade CI failures across all open PRs |
| 3 | date: 2026-05-16 |
| 4 | category: docs/solutions/workflow-issues |
| 5 | module: ci-release-engineering |
| 6 | problem_type: workflow_issue |
| 7 | component: testing_framework |
| 8 | severity: high |
| 9 | applies_when: |
| 10 | - a test asserts consistency between two release-time artifacts (e.g., SKILL.md version and a hardcoded pin in a shell script) |
| 11 | - one artifact is updated as part of a version bump and the other requires a manual lockstep update |
| 12 | - multiple long-lived PRs are open simultaneously against the same base branch |
| 13 | symptoms: |
| 14 | - every open PR's CI fails after a version bump even though the PRs are unrelated to versioning |
| 15 | - the failing test references a stale hardcoded value that was not updated alongside the bumped version |
| 16 | - PR authors must rebase and manually fix an artifact they did not touch |
| 17 | root_cause: missing_workflow_step |
| 18 | resolution_type: code_fix |
| 19 | related_components: |
| 20 | - development_workflow |
| 21 | - documentation |
| 22 | tags: |
| 23 | - ci |
| 24 | - release-engineering |
| 25 | - consistency-test |
| 26 | - version-pin |
| 27 | - cascade-failure |
| 28 | - test-design |
| 29 | - workflow |
| 30 | --- |
| 31 | |
| 32 | # Release-time consistency tests cause cascade CI failures across all open PRs |
| 33 | |
| 34 | ## Context |
| 35 | |
| 36 | A `tests/test_version_consistency.py::test_sync_cache_path_uses_skill_version` test was added to enforce that the version string embedded in `skills/last30days/scripts/sync.sh` (a hardcoded plugin-cache path segment) matched the version frontmatter in `skills/last30days/SKILL.md`. The intention was sound: the cache path had to stay in lockstep with the skill version or the sync would silently pull stale files. |
| 37 | |
| 38 | The test worked as designed until a release shipped. At that point it turned into a cascade-failure machine: |
| 39 | |
| 40 | 1. A release PR bumps `SKILL.md` version (e.g., 3.2.0 → 3.2.1) **and** bumps the `sync.sh` pin. That PR's CI is green. |
| 41 | 2. The release PR merges to `main`. |
| 42 | 3. Every PR that was open at merge time was branched from pre-release `main`. Those PRs have `SKILL.md` 3.2.1 (inherited via merge-base with `main`) but their branch never touched `sync.sh`. |
| 43 | 4. CI for those PRs runs the consistency test against the new `main` — `SKILL.md` says 3.2.1, `sync.sh` still says 3.2.0 — and fails. |
| 44 | 5. All open PRs are now red simultaneously, with a failure that has nothing to do with their changes. |
| 45 | |
| 46 | This affected at least five PRs during the 2026-05-13 to 2026-05-15 window: PR #400 (caught during rebase, required a manual pin bump), PRs #390 and #392 (OpenClaw `SCRAPECREATORS_API_KEY` fix, both stalled for the same stale-pin reason), and at least two others. A follow-up hotfix PR (#397 — `fix(sync): bump cache target to 3.2.1 to match SKILL.md`) was required just to unblock the queue. |
| 47 | |
| 48 | The permanent fix was PR #405: delete `sync.sh` entirely (the install workflow made it redundant) and drop `test_sync_cache_path_uses_skill_version`. Once both were gone, no version-consistency cascade was possible. |
| 49 | |
| 50 | ## Guidance |
| 51 | |
| 52 | ### 1. Don't write consistency tests that read two files and assert one matches a substring derived from the other |
| 53 | |
| 54 | This pattern looks safe but is not: |
| 55 | |
| 56 | ```python |
| 57 | def test_sync_cache_path_uses_skill_version(self) -> None: |
| 58 | sync_text = (SKILL_ROOT / "scripts" / "sync.sh").read_text(encoding="utf-8") |
| 59 | version = _skill_version() # reads SKILL.md |
| 60 | self.assertIn( |
| 61 | f'last30days-skill/last30days/{version}"', |
| 62 | sync_text, # asserts sync.sh contains that string |
| 63 | ) |
| 64 | ``` |
| 65 | |
| 66 | It encodes the assumption that both files are always updated together, in the same commit, on the same branch. That assumption breaks the moment two files have independent lifecycle owners — a versioned manifest and a deployment script are archetypal examples. |
| 67 | |
| 68 | ### 2. If the values genuinely need to stay in sync, derive one from the other at runtime |
| 69 | |
| 70 | Remove the hardcoded pin from `sync.sh` and compute it: |
| 71 | |
| 72 | ```bash |
| 73 | # sync.sh — derive version from SKILL.md at runtime, no pin to maintain |
| 74 | SKILL_VERSION=$(grep -m1 '^version:' "$(dirname "$0")/../SKILL.md" \ |
| 75 | | sed 's/version:[[:space:]]*"\([^"]*\)"/\1/') |
| 76 | CACHE_PATH="last30days-skill/last30days/${SKILL_VERSION}" |
| 77 | ``` |
| 78 | |
| 79 | Now there is only one source of truth (`SKILL.md`). The test that asserted they matched becomes vacuous and should be deleted. If `SKILL.md` is wrong, the sync itself will fail loudly — which is better feedback than a CI gate on a different PR. |
| 80 | |
| 81 | ### 3. If two values must stay independent for legitimate reasons, update them together and make the test self-skip if either source is missing |
| 82 | |
| 83 | If separate versioning is genuinely required (e.g., SKILL.md versions for harness consumers, sync.sh versions a private artifact store with its own cadence), update both in the same PR — never staggered — and write the test to self-skip rather than error when either file is absent: |
| 84 | |
| 85 | ```python |
| 86 | def test_sync_cache_path_uses_skill_version(self) -> None: |
| 87 | sync_sh = SKILL_ROOT / "scripts" / "sync.sh" |
| 88 | if not sync_sh.exists(): |
| 89 | self.skipTest("sync.sh not present; skipping pin consistency check") |
| 90 | sync_text = sync_sh.read_text(encoding="utf-8") |
| 91 | version = _skill_version() |
| 92 | self.assertIn( |
| 93 | f'last30days-skill/last30days/{version}"', |
| 94 | sync_text, |
| 95 | ) |
| 96 | ``` |
| 97 | |
| 98 | Self-skipping means deleting the file is a non-event in CI — no cascading red, no hotfix PR to the queue. |
| 99 | |
| 100 | ### 4. Run consistency tests against the merge-base diff, not main |
| 101 | |
| 102 | If you keep a two-file consistency test, scope it so it only fails when the PR itself modifies one of the two files but not the other. A GitHub Actions step can do this: |
| 103 | |
| 104 | ```yaml |
| 105 | - name: Check sync.sh version pin consistency |
| 106 | run: | |
| 107 | BASE=$(git merge-base HEAD origin/main) |
| 108 | SKILL_CHANGED=$(git diff --name-only "$BASE" HEAD | grep -c 'SKILL\.md' || true) |
| 109 | SYNC_CHANGED=$(git diff --name-only "$BASE" HEAD | grep -c 'sync\.sh' || true) |
| 110 | if [ "$SKILL_CHANGED" -gt 0 ] && [ "$SYNC_CHANGED" -eq 0 ]; then |
| 111 | echo "SKILL.md version bumped but sync.sh pin was not updated" |
| 112 | exit 1 |
| 113 | fi |
| 114 | ``` |
| 115 | |
| 116 | This only fires when your PR touched `SKILL.md` and left `sync.sh` alone — never because a release merged to `main` after you branched. |
| 117 | |
| 118 | ### 5. Ask whether you actually need this test |
| 119 | |
| 120 | If the values are wrong, downstream tooling will fail loudly: the sync will fetch the wrong artifact, installs will break, or the harness will reject the version. A test that exists only to catch a human-bookkeeping error at release time adds cascade-fail risk without offering a meaningfully earlier signal. Weigh that cost before adding any two-file consistency gate. |
| 121 | |
| 122 | ## Why This Matters |
| 123 | |
| 124 | The damage from a stale-pin consistency test is asymmetric. It: |
| 125 | |
| 126 | - Fails on every open PR simultaneously the moment a release lands on `main` — not just the PR that forgot to update the pin. |
| 127 | - Produces a failure message that points at a line in a test file with no obvious relationship to the PR's actual changes. |
| 128 | - Requires either a hotfix PR (touching a file the failing PRs have no business touching) or a manual rebase of every affected branch. |
| 129 | - Blocks work that has already been reviewed and approved. |
| 130 | |
| 131 | In this repo the effect was measurable: at least five PRs stalled across a two-day window, one hotfix PR was shipped just to unblock the queue, and multiple authors spent time debugging a failure completely unrelated to their changes. |
| 132 | |
| 133 | The broader principle is that tests which gate on *bookkeeping consistency between files* impose their maintenance cost on every contributor, every time, even when those contributors did nothing wrong. That cost compounds with team size and release cadence. |
| 134 | |
| 135 | ## When to Apply |
| 136 | |
| 137 | Apply this guidance whenever you find yourself: |
| 138 | |
| 139 | - Writing a test that reads two files and asserts that a string in one matches a value derived from the other. |
| 140 | - Adding a CI step labeled "consistency check," "sync check," or "pin check" where the check compares a hardcoded value against a computed one from a separate file. |
| 141 | - Working in a repo where a versioned manifest (e.g., `SKILL.md`, `package.json`, `pyproject.toml`) and a deployment artifact (e.g., a shell script, a Dockerfile, a Helm values file) are both maintained by hand. |
| 142 | - Reviewing a PR that touches only one of two "paired" files and fails a consistency test for the other. |
| 143 | |
| 144 | It does *not* apply to tests that read a single source of truth and validate its internal structure (e.g., asserting that `SKILL.md`'s frontmatter version is double-quoted, or that `package.json`'s `version` field is a valid semver string). Those tests have one file and one assertion; they cannot cascade across branches. |
| 145 | |
| 146 | ## Examples |
| 147 | |
| 148 | ### Before — the pattern that caused the cascade |
| 149 | |
| 150 | Original `tests/test_version_consistency.py` (deleted in commit `9fb19ea`): |
| 151 | |
| 152 | ```python |
| 153 | import re |
| 154 | import unittest |
| 155 | from pathlib import Path |
| 156 | |
| 157 | ROOT = Path(__file__).resolve().parents[1] |
| 158 | SKILL_ROOT = ROOT / "skills" / "last30days" |
| 159 | |
| 160 | |
| 161 | def _skill_version() -> str: |
| 162 | text = (SKILL_ROOT / "SKILL.md").read_text(encoding="utf-8") |
| 163 | match = re.search(r'^version:\s*"([^"]+)"\s*$', text, re.MULTILINE) |
| 164 | if not match: |
| 165 | raise AssertionError("SKILL.md version frontmatter not found") |
| 166 | return match.group(1) |
| 167 | |
| 168 | |
| 169 | class TestVersionConsistency(unittest.TestCase): |
| 170 | def test_sync_cache_path_uses_skill_version(self) -> None: |
| 171 | sync_text = (SKILL_ROOT / "scripts" / "sync.sh").read_text(encoding="utf-8") |
| 172 | version = _skill_version() # source 1: SKILL.md frontmatter |
| 173 | self.assertIn( # assertion: sync.sh must contain |
| 174 | f'last30days-skill/last30days/{version}"', |
| 175 | sync_text, # source 2: hardcoded string in sync.sh |
| 176 | ) |
| 177 | ``` |
| 178 | |
| 179 | `sync.sh` contained a line like: |
| 180 | |
| 181 | ```bash |
| 182 | PLUGIN_CACHE="$HOME/.cache/last30days-skill/last30days/3.2.0" |
| 183 | ``` |
| 184 | |
| 185 | When SKILL.md bumped to `3.2.1` in a release PR, `sync.sh` was updated in the same PR and CI stayed green. But every PR branched before that release still had `sync.sh` at `3.2.0`. Their CI failed immediately, with an assertion error pointing at the test, not at the release PR. |
| 186 | |
| 187 | ### After — what we did: delete both |
| 188 | |
| 189 | PR #405 deleted `sync.sh` (the install workflow replaced it) and dropped `test_sync_cache_path_uses_skill_version` in the same change. No consistency gate, no pin to maintain, no cascade possible. |
| 190 | |
| 191 | ### After — what we could have done instead: derive at runtime |
| 192 | |
| 193 | If `sync.sh` had still been needed, the right fix would have been to remove the hardcoded version from the script and derive it from `SKILL.md`: |
| 194 | |
| 195 | ```bash |
| 196 | #!/usr/bin/env bash |
| 197 | # sync.sh — no hardcoded version; reads SKILL.md as single source of truth |
| 198 | SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" |
| 199 | SKILL_VERSION=$(grep -m1 '^version:' "${SCRIPT_DIR}/../SKILL.md" \ |
| 200 | | sed 's/version:[[:space:]]*"\([^"]*\)"/\1/') |
| 201 | |
| 202 | if [ -z "$SKILL_VERSION" ]; then |
| 203 | echo "error: could not parse version from SKILL.md" >&2 |
| 204 | exit 1 |
| 205 | fi |
| 206 | |
| 207 | PLUGIN_CACHE="$HOME/.cache/last30days-skill/last30days/${SKILL_VERSION}" |
| 208 | # ... rest of sync logic |
| 209 | ``` |
| 210 | |
| 211 | With this in place, `test_sync_cache_path_uses_skill_version` has no reason to exist — there is nothing to assert. Delete it. If the version parsing breaks, `sync.sh` itself exits non-zero with a clear message. |
| 212 | |
| 213 | ## Related |
| 214 | |
| 215 | - **PR #397** (merged) — `fix(sync): bump cache target to 3.2.1 to match SKILL.md`. The hotfix that unblocked the cascade temporarily by bumping the pin. |
| 216 | - **PR #400** (merged) — caught the same cascade during rebase; had to bump the pin to clear CI. |
| 217 | - **PR #390** (closed) and **PR #392** (rebased + merged) — OpenClaw `SCRAPECREATORS_API_KEY` fix; both blocked by the cascade until rebased onto post-#405 main. |
| 218 | - **PR #405** (merged) — the permanent fix: deleted `sync.sh` + `test_sync_cache_path_uses_skill_version` together. |
| 219 | - **PR #412** (merged) — adjacent work that consolidated SKILL.md version parsing into `lib/skill_meta.py`, reducing future drift risk by giving the version field one canonical reader. |
| 220 |