From 92b9e6b01feae14693daf20bb086350f1be13d0c Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 22 Aug 2026 01:31:36 -0400 Subject: [PATCH 1/2] ci(quality): stop push-runs failing on missing PR-context secrets (#402) The tokens were created on 2026-08-18, the preflight's single 'configured' flag flipped true, both scans ran for the first time -- and both failed on ORG-level prerequisites no repository secret can carry: SonarQube Cloud's mandatory sonar.organization (still commented out in sonar-project.properties) and the Snyk organization (the server answered every scan with a 422). Every push to main went red for reasons the repository could not fix -- exactly the permanently-red main this preflight exists to prevent. Readiness is now measured PER SCANNER and beyond token presence: preflight outputs sonar_ready (token + active sonar.organization in sonar-project.properties) and snyk_ready (token + the SNYK_ORG repository variable, passed as --org), each job gated on its own output, so each scan lights up the moment its missing piece appears. Dispatch still fails loudly; push/schedule skip with a run summary naming what to set and where. The snyk monitor --org placeholder becomes that variable wiring. --- .github/workflows/code-quality.yml | 206 +++++++++++++++++++++-------- tests/test_security_scans.py | 95 +++++++++++-- 2 files changed, 232 insertions(+), 69 deletions(-) diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index ac264ddb..444ade98 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -4,22 +4,34 @@ name: Code Quality # # NOTE (#291): the tokenless baseline now lives in `security.yml` -- Dependabot, a weekly # `pip-audit` over the exported lock, and CodeQL, all on GITHUB_TOKEN alone, always on. -# This workflow is the OPTIONAL enhanced tier: SonarQube and Snyk run only if the two -# secrets below are ever created, and its `preflight` skips them cleanly (never red) while -# they are not. The two workflows disagree about nothing: baseline there, depth here. -# ┌─ BEFORE THIS CAN RUN ───────────────────────────────────────────────────────────────────────── -# │ Two repository secrets must exist. As of 2026-08-16, NEITHER DOES -- `gh secret list` is empty -# │ at both repo and org level: +# This workflow is the OPTIONAL enhanced tier: SonarQube and Snyk run only when everything +# each scan needs is configured -- which is MORE than the tokens (see #402 below) -- and +# the `preflight` skips whichever scan is not ready, cleanly (never red), until it is. +# The two workflows disagree about nothing: baseline there, depth here. +# ┌─ BEFORE EACH SCAN CAN RUN ─────────────────────────────────────────────────────────────────── +# │ The two repository secrets were created on 2026-08-18. From that moment every push to `main` +# │ failed (#402): the old preflight treated "both tokens exist" as "both scans can run", the +# │ scans started for real, and each died on a prerequisite that no repository secret carries: # │ -# │ SONAR_TOKEN -- from SonarQube ("My Account" -> Security -> Generate Token) -# │ SNYK_TOKEN -- from Snyk (Account Settings -> Auth Token, or `snyk config get api`) +# │ SONAR_TOKEN -- exists. SonarQube Cloud still refuses the analysis with +# │ "You must define the following mandatory properties ...: +# │ sonar.organization" until `sonar.organization` is ACTIVE in +# │ `sonar-project.properties`. It is deliberately left commented +# │ out there until the org owner sets the real key, because a +# │ wrong key fails with a misleading "project not found". +# │ sonar.organization -- SonarCloud > organization settings; set it in +# │ `sonar-project.properties` at the repo root. +# │ SNYK_TOKEN -- exists. Snyk still rejects the scan server-side (422, +# │ SNYK-OS-PYTHON-0013) until the Snyk organization exists and is +# │ wired to this project -- org-level setup on snyk.io. +# │ SNYK_ORG -- a repository VARIABLE (Settings > Secrets and variables > +# │ Actions > Variables) holding the Snyk organization id; the +# │ scans pass it as `--org`. Declared a variable, not a secret, +# │ because an organization id is not sensitive. # │ -# │ Set both at https://github.com/CodeGateSoftware/keel/settings/secrets/actions. -# │ `sonar-project.properties` at the repo root also carries a `sonar.projectKey` that must match -# │ the project as it exists on the Sonar server, and a commented-out `sonar.organization` that -# │ SonarQube Cloud requires -- read that file before the first run. -# │ -# │ UNTIL THEY EXIST, this workflow does NOT fail the build. It skips both scans and explains +# │ The preflight below checks every one of these per scanner, so `main` is green now and each +# │ scan lights up THE MOMENT its own missing piece appears -- no coordinated flag day. +# │ UNTIL THEN, this workflow does NOT fail the build. It skips what is not ready and explains # │ itself in the run summary -- unless a human DISPATCHED it, in which case it fails loudly, # │ because silently ignoring a direct request is worse. See `preflight` for the full argument. # └─────────────────────────────────────────────────────────────────────────────────────────────── @@ -59,74 +71,145 @@ permissions: contents: read jobs: - # Both scanners fail in unhelpful ways when their token is missing -- Sonar reports "You're not - # authorized. Please check the property sonar.token", and Snyk exits with a generic auth error - # from inside a container. Neither says "the repository secret does not exist", which is the - # actual cause and the only thing the reader needs to know. This job says it once, up front. + # Both scanners fail in unhelpful ways when something they need is missing -- Sonar reports + # "You must define the following mandatory properties ...: sonar.organization", and Snyk exits + # with a generic 422 from inside the CLI. Neither says which repository or organization setting + # is the actual cause and the only thing the reader needs to know. This job says it once, up + # front, PER SCANNER. + # + # #402 is why readiness is measured per scanner and beyond token presence. When the workflow + # was written no secret existed, so "are both tokens set?" was an honest readiness check and + # one shared `configured` flag was safe. The tokens appeared on 2026-08-18, the flag flipped + # true, both scans ran for the first time -- and both failed, because each still lacked an + # ORGANIZATION-level prerequisite (SonarCloud's mandatory `sonar.organization`; the Snyk + # organization the project must be filed under). Every push to `main` went red for reasons + # the repository could not fix: exactly the permanently-red-main this preflight exists to + # prevent. The check is now: per scanner, everything that scan actually needs. # # ⚠️ IT SAYS IT DIFFERENTLY DEPENDING ON WHO ASKED, and that asymmetry is the whole point: # # workflow_dispatch -> a human asked for a scan. If it cannot run, FAIL, loudly. Silently # doing nothing in response to a direct request is the worse outcome. - # push / schedule -> nobody asked; the trigger fired on its own. If the tokens are not - # configured, SKIP cleanly and say so in the run summary. + # push / schedule -> nobody asked; the trigger fired on its own. If a scan is not fully + # configured, SKIP it cleanly and say so in the run summary. # - # The second half exists because `push: [main]` was added in #272 when the repo went public and - # Actions minutes stopped being billed. Before that this workflow ran weekly, so an unconfigured - # repo produced one red X every Monday. On every merge, the same behaviour is a permanently red - # `main` that teaches the reader to ignore CI -- which costs more than the missing scan does. - # A skipped scan that announces itself is honest; a red X nobody reads is not. + # `push: [main]` was added in #272 when the repo went public and Actions minutes stopped being + # billed, and a skipped scan that announces itself is still the right behaviour there: a red X + # on every merge teaches the reader to ignore CI, which costs more than the missing scan does. # # `secrets` cannot be referenced from a job-level `if:`, which is why this is an OUTPUT consumed - # by the two scan jobs rather than a condition written directly on them. + # by the two scan jobs rather than a condition written directly on them. The outputs are + # SEPARATE (`sonar_ready`, `snyk_ready`) so each scan lights up the moment its own + # prerequisites are in place -- one shared flag would keep Snyk dark over a Sonar gap, and + # vice versa. preflight: - name: Check required secrets + name: Check each scan is actually configured runs-on: ubuntu-latest timeout-minutes: 5 outputs: - configured: ${{ steps.check.outputs.configured }} + sonar_ready: ${{ steps.check.outputs.sonar_ready }} + snyk_ready: ${{ steps.check.outputs.snyk_ready }} steps: - - name: Verify SONAR_TOKEN and SNYK_TOKEN are set + # The Sonar check reads `sonar-project.properties` from the repository. Default shallow + # clone is enough; nothing from the repo is executed here. + - name: Checkout code + uses: actions/checkout@v7 + + - name: Verify SonarQube and Snyk are each ready to run id: check env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }} + SNYK_ORG: ${{ vars.SNYK_ORG }} run: | set -euo pipefail - MISSING="" - [ -n "${SONAR_TOKEN:-}" ] || MISSING="$MISSING SONAR_TOKEN" - [ -n "${SNYK_TOKEN:-}" ] || MISSING="$MISSING SNYK_TOKEN" + if [ ! -f sonar-project.properties ]; then + # If the grep below simply found nothing, the Sonar scan would be skipped FOREVER, + # on every event, with the workflow claiming to be "not configured yet". Fail + # loudly instead -- a missing file is a broken workflow, not a missing setting. + echo "::error::preflight cannot read sonar-project.properties -- is the checkout step gone?" + exit 1 + fi - if [ -z "$MISSING" ]; then - echo "configured=true" >> "$GITHUB_OUTPUT" - echo "SONAR_TOKEN and SNYK_TOKEN are both present." - exit 0 + # Each scanner's list is measured against what THAT scan actually needs (#402). + SONAR_MISSING="" + SNYK_MISSING="" + + [ -n "${SONAR_TOKEN:-}" ] || SONAR_MISSING="$SONAR_MISSING SONAR_TOKEN" + # SonarQube Cloud refuses the analysis without an active `sonar.organization`. It is + # commented out in sonar-project.properties until the org owner fills in the real + # key -- a wrong key fails with a misleading "project not found" -- so "the line is + # still commented" genuinely means "not ready", and the scan must not run. + grep -Eq '^[[:space:]]*sonar\.organization[[:space:]]*=' sonar-project.properties \ + || SONAR_MISSING="$SONAR_MISSING sonar.organization" + + [ -n "${SNYK_TOKEN:-}" ] || SNYK_MISSING="$SNYK_MISSING SNYK_TOKEN" + # The Snyk organization to file the project under, passed as `--org` below. A + # repository VARIABLE rather than a secret: an organization id is not sensitive, + # and the preflight needs to branch on it, which secrets cannot do from a job `if:`. + [ -n "${SNYK_ORG:-}" ] || SNYK_MISSING="$SNYK_MISSING SNYK_ORG" + + if [ -z "$SONAR_MISSING" ]; then + echo "sonar_ready=true" >> "$GITHUB_OUTPUT" + else + echo "sonar_ready=false" >> "$GITHUB_OUTPUT" + fi + if [ -z "$SNYK_MISSING" ]; then + echo "snyk_ready=true" >> "$GITHUB_OUTPUT" + else + echo "snyk_ready=false" >> "$GITHUB_OUTPUT" fi - echo "configured=false" >> "$GITHUB_OUTPUT" + if [ -z "$SONAR_MISSING" ] && [ -z "$SNYK_MISSING" ]; then + echo "SonarQube and Snyk are both fully configured." + exit 0 + fi + MISSING="$SONAR_MISSING$SNYK_MISSING" { echo "### Code quality scans skipped" echo - echo "These repository secrets do not exist, so SonarQube and Snyk cannot run:" - echo - for s in $MISSING; do echo "- \`$s\`"; done - echo - echo "Add them at [Settings > Secrets > Actions](https://github.com/${{ github.repository }}/settings/secrets/actions):" - echo - echo "| secret | where to get it |" - echo "| --- | --- |" - echo "| \`SONAR_TOKEN\` | SonarQube: My Account > Security > Generate Token |" - echo "| \`SNYK_TOKEN\` | Snyk: Account Settings > Auth Token (or \`snyk config get api\`) |" + echo "Something these scans need is not configured, so the unready one(s) were" + echo "skipped. This is not a build failure." + if [ -n "$SONAR_MISSING" ]; then + echo + echo "**SonarQube scan** still needs:" + for s in $SONAR_MISSING; do + case "$s" in + SONAR_TOKEN) + echo "- \`SONAR_TOKEN\` -- repository secret. SonarQube: My Account > Security >" + echo " Generate Token; add it at [Settings > Secrets > Actions](https://github.com/${{ github.repository }}/settings/secrets/actions)" ;; + sonar.organization) + echo "- \`sonar.organization\` -- uncomment/set it in \`sonar-project.properties\`" + echo " to the SonarCloud organization key. SonarQube Cloud rejects the scan" + echo " without it, even with a valid token" ;; + esac + done + fi + if [ -n "$SNYK_MISSING" ]; then + echo + echo "**Snyk dependency scan** still needs:" + for s in $SNYK_MISSING; do + case "$s" in + SNYK_TOKEN) + echo "- \`SNYK_TOKEN\` -- repository secret. Snyk: Account Settings > Auth Token" + echo " (or \`snyk config get api\`); add it at [Settings > Secrets > Actions](https://github.com/${{ github.repository }}/settings/secrets/actions)" ;; + SNYK_ORG) + echo "- \`SNYK_ORG\` -- repository VARIABLE at [Settings > Secrets and variables > Actions](https://github.com/${{ github.repository }}/settings/variables/actions):" + echo " the Snyk organization id the project is filed under (the scans pass it" + echo " as \`--org\`). The Snyk organization itself is created on snyk.io" ;; + esac + done + fi } >> "$GITHUB_STEP_SUMMARY" if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then # A human asked for this scan. Refusing quietly would be worse than failing. - echo "::error title=Missing repository secrets::You dispatched this workflow, but it cannot run until these secrets exist:$MISSING" + echo "::error title=Scans not configured::You dispatched this workflow, but it cannot run until these exist:$MISSING" exit 1 fi - echo "::notice title=Code quality scans skipped::Not configured yet -- missing:$MISSING. This is not a build failure; see the run summary." + echo "::notice title=Code quality scans skipped::Not fully configured yet -- missing:$MISSING. This is not a build failure; see the run summary." # ONE whole-repo scan, not the reference's per-module matrix. The reference fans out over a # Node monorepo whose modules each have their own package.json, tsconfig and test run, and it @@ -139,7 +222,10 @@ jobs: sonarqube: name: SonarQube scan needs: preflight - if: needs.preflight.outputs.configured == 'true' + # Own readiness output, not a shared flag (#402): the token exists but SonarQube Cloud + # still refuses the scan until `sonar.organization` is set, and this job must stay dark + # until that appears -- while running the moment it does. + if: needs.preflight.outputs.sonar_ready == 'true' runs-on: ubuntu-latest timeout-minutes: 20 steps: @@ -178,7 +264,10 @@ jobs: snyk: name: Snyk dependency scan needs: preflight - if: needs.preflight.outputs.configured == 'true' + # Own readiness output (#402): runs only when the token AND the Snyk organization + # (`SNYK_ORG`) are configured -- the organization is what the Snyk side was missing when + # every push to main failed with a server-side 422. + if: needs.preflight.outputs.snyk_ready == 'true' runs-on: ubuntu-latest timeout-minutes: 15 steps: @@ -229,24 +318,25 @@ jobs: uses: snyk/actions/setup@v1 # Fails the job on findings. Add `--severity-threshold=high` to only fail on high/critical. + # `--org` files the project under the SNYK_ORG organization -- the same variable the + # preflight gates this job on, so a scan that runs is always a scan that lands somewhere. - name: Snyk test env: SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }} - run: snyk test --file=requirements.txt --package-manager=pip + SNYK_ORG: ${{ vars.SNYK_ORG }} + run: snyk test --org="$SNYK_ORG" --file=requirements.txt --package-manager=pip # Reports the current dependency tree to the Snyk dashboard so newly-published CVEs against # these exact pins raise an alert between weekly runs. `always()` so a failing `snyk test` # above still leaves the dashboard up to date -- the finding is the reason to record it. + # Same `--org` as the test: a monitor filed under a different organization than the one + # tested would split the project's history across two dashboards. - name: Snyk monitor if: always() env: SNYK_TOKEN: ${{ secrets.SNYK_TOKEN }} - # PLACEHOLDER -- SET BEFORE FIRST USE, OR DELETE: - # Add `--org=` below to file this project under a specific Snyk - # organization. It is deliberately omitted rather than copied from the workflow this was - # modelled on, whose `--org` value belongs to an unrelated project; with no `--org`, Snyk - # uses the token's default organization, which is correct for a single-org account. - run: snyk monitor --file=requirements.txt --package-manager=pip --project-name=keel + SNYK_ORG: ${{ vars.SNYK_ORG }} + run: snyk monitor --org="$SNYK_ORG" --file=requirements.txt --package-manager=pip --project-name=keel # NO DOCKER SCAN JOB, on purpose. The reference workflow builds an image and runs # `snyk/actions/docker` against it. keel has no Dockerfile and ships no container -- it is diff --git a/tests/test_security_scans.py b/tests/test_security_scans.py index af7085e8..410f3a80 100644 --- a/tests/test_security_scans.py +++ b/tests/test_security_scans.py @@ -15,7 +15,16 @@ `uv export --frozen` (the lock as committed, never a fresh resolve), and the secrets rule is the honest one: the always-on scan workflows reference no secrets at all, and the optional tier may reference `SONAR_TOKEN`/`SNYK_TOKEN` ONLY from jobs a preflight guards --- so a missing token can never redden a scheduled run, only skip it with an explanation. +-- so a missing prerequisite can never redden a scheduled run, only skip it with an +explanation. + +And #402 taught where "configured" stops: the tokens were created on 2026-08-18, the +preflight flipped to ready, both scans ran for the first time -- and both FAILED, because +each needs ORG-level configuration no repository secret can carry (SonarQube Cloud's +mandatory `sonar.organization`; the Snyk organization on the snyk.io side). Every push to +`main` went red for reasons the repository could not fix, which is the exact permanently- +red-main outcome this tier was designed never to produce. The guard is therefore pinned +PER SCANNER and must measure the scan's real readiness, not merely token existence. """ from __future__ import annotations @@ -189,18 +198,28 @@ def test_the_optional_tier_only_asks_for_its_tokens_behind_the_preflight_guard() """code-quality.yml may name SONAR_TOKEN/SNYK_TOKEN -- but only guarded. The optional tier's design constraint (its own header documents it): a job that needs - a token must declare `needs: preflight` and run under `if: needs.preflight.outputs. - configured == 'true'`, so a missing token SKIPS with an explanation instead of - reddening every scheduled run. This test pins the guard structurally, per job, so a - future edit cannot detach a token-referencing job from its preflight. + a token must declare `needs: preflight` and run under the readiness guard FOR ITS OWN + scanner, so a missing prerequisite SKIPS with an explanation instead of reddening + every push to main. #402 is what happens when the guard is coarser than the failure: + one shared `configured` flag was true the moment both tokens existed, so both scan + jobs started running while only one of each pair's prerequisites was in place. This + test pins the guard structurally, per job, so a future edit cannot detach a + token-referencing job from its preflight -- nor re-merge the two scanners into one + all-or-nothing flag. """ workflow = yaml.safe_load((_WORKFLOWS / _OPTIONAL_TIER).read_text()) jobs = workflow.get("jobs", {}) assert "preflight" in jobs, ( f"{_OPTIONAL_TIER} must keep its preflight job -- it is what lets the optional " - "tier skip cleanly while the tokens do not exist" + "tier skip cleanly while its prerequisites are missing" ) - guard = "needs.preflight.outputs.configured == 'true'" + outputs = jobs["preflight"].get("outputs", {}) + for output in ("sonar_ready", "snyk_ready"): + assert output in outputs, ( + f"preflight must declare a per-scanner `{output}` output -- a guard named in " + "a job's `if` but never produced skips that job FOREVER, silently" + ) + token_to_guard = {"SONAR_TOKEN": "sonar_ready", "SNYK_TOKEN": "snyk_ready"} for name, job in jobs.items(): serialized = str(job) referenced = [s for s in _FORBIDDEN_IN_BASELINE if f"secrets.{s}" in serialized] @@ -210,10 +229,64 @@ def test_the_optional_tier_only_asks_for_its_tokens_behind_the_preflight_guard() f"{_OPTIONAL_TIER}'s job {name!r} references {referenced} but does not declare " "`needs: preflight` -- a token-referencing job must be guarded" ) - assert job.get("if") == guard, ( - f"{_OPTIONAL_TIER}'s job {name!r} references {referenced} but is not gated on " - f"`{guard}` -- without the guard a missing token reddens every scheduled run" - ) + for token in referenced: + guard = f"needs.preflight.outputs.{token_to_guard[token]} == 'true'" + assert guard in str(job.get("if", "")), ( + f"{_OPTIONAL_TIER}'s job {name!r} references {token} but is not gated on " + f"`{guard}` -- without its own guard a missing prerequisite for that " + "scanner reddens every push to main (#402)" + ) + + +def test_preflight_readiness_includes_the_org_level_config_not_just_the_tokens(): + """The #402 lesson: a token proves the secret exists, not that the scan can run. + + Both tokens were created on 2026-08-18; from that push onward every run of this + workflow on `main` failed -- SonarQube Cloud with "You must define the following + mandatory properties ... sonar.organization", Snyk with a server-side 422 -- because + the remaining prerequisites live OUTSIDE the repository: the `sonar.organization` + property (deliberately commented out in sonar-project.properties until the org owner + supplies the real key) and the Snyk organization that the scan must be filed under + (the `--org` the snyk monitor step has carried as a placeholder since the workflow + was written). A preflight that checks only token presence cannot see either one, so + this test pins that it checks both, and that each scan passes its org along when it + finally runs. + """ + workflow = yaml.safe_load((_WORKFLOWS / _OPTIONAL_TIER).read_text()) + preflight_text = str(workflow["jobs"]["preflight"]) + assert "sonar.organization" in preflight_text, ( + "preflight must check sonar-project.properties carries an active " + "`sonar.organization` -- SonarQube Cloud rejects the scan without it (#402), and " + "a token-only check stays green while the scan cannot run" + ) + assert "SNYK_ORG" in preflight_text, ( + "preflight must check the SNYK_ORG repository variable -- the Snyk organization " + "is org-level configuration the repository cannot derive, and the 422 it causes " + "is invisible to a token-only check (#402)" + ) + snyk_text = str(workflow["jobs"]["snyk"]) + assert "SNYK_ORG" in snyk_text and "--org" in snyk_text, ( + "the snyk job must pass the configured organization (`--org`) -- a placeholder " + "comment is not configuration; the scan must be filed under the real org" + ) + + +def test_preflight_still_fails_loudly_only_when_a_human_dispatched_it(): + """The asymmetry that makes skipping honest: a human dispatch is refused loudly, + an automatic trigger skips with an explanation. Reversed, the workflow either + reddens every push to main while prerequisites are missing (#402, observed) or + silently ignores a direct request.""" + workflow = yaml.safe_load((_WORKFLOWS / _OPTIONAL_TIER).read_text()) + steps = workflow["jobs"]["preflight"]["steps"] + run = "\n".join(str(step.get("run", "")) for step in steps) + assert "workflow_dispatch" in run, ( + "preflight must branch on the event type -- the skip-vs-fail decision is the " + "point of the preflight job" + ) + assert "::error" in run and "exit 1" in run, ( + "a dispatched run that cannot scan must FAIL with an annotation -- silently " + "doing nothing in response to a direct request is the worse outcome" + ) def test_the_security_workflow_runs_on_a_schedule_not_only_on_push(): From 21d3e3b86b15146c9e0446117ffdc3b44ad25fe1 Mon Sep 17 00:00:00 2001 From: Elmehdi Aitbrahim Date: Sat, 22 Aug 2026 01:31:42 -0400 Subject: [PATCH 2/2] ci(release): fail a stale uv.lock early, naming the remedy (#424) The 0.10.0 release failed five steps after the mistake: #422 bumped seven pyproject.toml files and not the tracked uv.lock; 'uv sync' silently re-locked the checkout, the stamp recorded a dirty tree, and the reader was sent chasing 'artifact reports a dirty tree' through keel/version.py and the stamp step instead of the lockfile -- after lint, types, tests and a full build had all run and passed. 'The lockfile must already be current' now runs right after the interpreter is in place and immediately before 'uv sync' (the first step that can mutate the tree): uv lock --check re-resolves against the manifests, exits non-zero WITHOUT writing, and the failure message names the remedy -- run uv lock locally and commit the result with the version bump. RELEASING.md step 1 now says the bump is pyproject.toml AND uv lock. --- .github/workflows/release.yml | 15 ++++++++++ docs/RELEASING.md | 8 ++++-- tests/test_desktop_packaging.py | 49 +++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1d25a8d3..116e249c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -58,6 +58,21 @@ jobs: - name: Set up Python run: uv python install # version comes from .python-version; never pin it here twice + # #424: the 0.10.0 release failed five steps late because a version bump moved seven + # pyproject.toml files and not the eighth thing -- uv.lock. `uv sync` below silently + # re-locks a stale checkout, the stamp then records a dirty tree, and the failure only + # surfaces as "artifact reports a dirty tree", pointing at keel/version.py and the + # stamp step instead of the lockfile, after lint/types/tests/build have all run. + # `--check` re-resolves against the manifests and exits non-zero WITHOUT writing, so + # this must come before anything that can mutate the tree. + - name: The lockfile must already be current + run: | + set -euo pipefail + if ! uv lock --check; then + echo "::error::uv.lock is stale for this pyproject -- run 'uv lock' locally and commit the result with the version bump. (A stale lock is otherwise discovered five steps later as 'artifact reports a dirty tree'.)" + exit 1 + fi + - name: Sync dependencies run: uv sync --all-extras --dev diff --git a/docs/RELEASING.md b/docs/RELEASING.md index ca66186f..8b55e802 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -25,13 +25,17 @@ the wheels it just built. ## Cutting a release -1. **Bump the version in a reviewed PR.** Edit `version` in `pyproject.toml`. The release workflow +1. **Bump the version in a reviewed PR.** Edit `version` in `pyproject.toml`, then run `uv lock` + and commit the refreshed `uv.lock` with it — the bump moves every workspace `pyproject.toml`, + and the lockfile is the one more thing that must move with them (#424; the workflow checks it + before anything else and fails the release if it was left behind). The release workflow refuses to change the version itself — that decision belongs in a PR a human reviewed, so CI never writes to `main`. (The **first** release needs no bump: `pyproject.toml` already says `0.1.0`.) 2. **Merge it**, then **Actions → Release → Run workflow**, entering the same version. 3. The workflow: validates the input is semver and matches `pyproject.toml` and no such tag exists - → runs tests + ruff → stamps the commit into `keel/_build_info.py` → `uv build --all-packages` + → checks `uv.lock` is current → runs tests + ruff → stamps the commit into + `keel/_build_info.py` → `uv build --all-packages` → installs the wheel into a clean venv **by path** and asserts it self-identifies as a clean `[release]` → verifies the live config asset is `mode: confirm` → tags `v` → composes release notes → publishes the GitHub Release with all wheels **and `config.yaml`** attached. diff --git a/tests/test_desktop_packaging.py b/tests/test_desktop_packaging.py index 398ab804..7ef7a9b9 100644 --- a/tests/test_desktop_packaging.py +++ b/tests/test_desktop_packaging.py @@ -64,6 +64,55 @@ def _steps_text(job: dict) -> str: return "\n".join(str(step.get("run", "")) for step in job["steps"]) +@pytest.fixture(scope="module") +def release_job(workflow: dict) -> dict: + return workflow["jobs"]["release"] + + +# -- the release must fail on a stale lockfile where the mistake is made ------------------------ + + +def test_the_lockfile_is_checked_before_anything_can_mutate_it( + workflow: dict, release_job: dict +) -> None: + """#424: the 0.10.0 release, and the error that pointed everywhere but at the cause. + + A version bump touches seven pyproject.toml files; `uv.lock` is the eighth thing that + must move with them. When it does not, `uv sync` silently re-locks the checkout, the + release is stamped from a dirty tree, and the failure surfaces FIVE steps later as + "artifact reports a dirty tree" -- after lint, types, tests and a full build have all + run and passed, sending the reader to `keel/version.py` and the stamp step rather + than to the lockfile. The guard belongs before the first step that can mutate the + tree (`uv sync`), and its failure message must NAME the remedy. + """ + steps = release_job["steps"] + names = [str(step.get("name", "")) for step in steps] + lock_step = next( + (i for i, s in enumerate(steps) if "uv lock --check" in str(s.get("run", ""))), + None, + ) + assert lock_step is not None, ( + "the release must run `uv lock --check` before it can spend minutes discovering " + "a stale lockfile as a dirty-tree error (#424)" + ) + assert "set -euo pipefail" in str(steps[lock_step].get("run", "")), ( + "the lock check must fail the step outright, not fall through to a later one" + ) + run = str(steps[lock_step].get("run", "")) + assert "stale" in run and "uv lock" in run and "::error" in run, ( + "the lock check's failure message must name the cause (stale uv.lock) and the " + "remedy (run `uv lock` locally and commit it with the version bump) -- the " + "misdirecting error is the reason this guard exists" + ) + for later in ("Sync dependencies", "Test"): + at = names.index(later) + assert lock_step < at, ( + f"the lock check must run before {later!r} -- `uv sync` is the step that " + "silently re-locks a stale checkout, and everything after it builds on a " + "tree the release did not intend to ship" + ) + + # -- the thing that must not happen ------------------------------------------------------------