diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml index a2d0b71..55473d4 100644 --- a/.github/actionlint.yaml +++ b/.github/actionlint.yaml @@ -6,3 +6,27 @@ self-hosted-runner: labels: - mdb-dev - mdb-prod + +# The reusables check their own scripts and composite out at `job.workflow_sha`, +# the commit of the workflow file that defines the running job. That is what makes +# a consumer's `@` pin mean anything, because pinning the workflow while its +# scripts float on `main` is not a pin. +# +# actionlint 1.7.12 does not know the `job.workflow_*` properties yet. Its `job` +# context model is `{check_run_id, container, services, status}`, and GitHub added +# `workflow_ref`, `workflow_sha`, `workflow_repository` and `workflow_file_path` +# in April 2026, after that release. The ignore is scoped to the one property name +# so nothing else is muted, and it should be deleted once actionlint catches up. +# +# Do NOT reinstate the previous form of this ignore, which muted +# `property "job_workflow_sha" is not defined`. That error was TRUE: +# `github.job_workflow_sha` is an OIDC token claim and has never existed in the +# `github` context (actions/runner#2417, "only supported as an OIDC claim"), so it +# evaluated to empty and `actions/checkout` silently took the default branch — +# exactly the un-pinned behaviour the comment above claimed to prevent. The tell +# was that actionlint already carried the newer `github.workflow_ref` and +# `github.workflow_sha`; it was not behind, the property was not real. +paths: + .github/workflows/**.yml: + ignore: + - 'property "workflow_sha" is not defined' diff --git a/.github/workflows/notify-main-failure.yml b/.github/workflows/notify-main-failure.yml index 01f9a97..08467ae 100644 --- a/.github/workflows/notify-main-failure.yml +++ b/.github/workflows/notify-main-failure.yml @@ -1,47 +1,52 @@ # Reusable workflow: post a Slack alert to the engineering channel when a -# workflow that runs on push to main/staging (or the release-train -# freeze/unfreeze cycle) FAILS, and a follow-up when it RECOVERS. +# workflow that runs on push to main/staging FAILS, and a follow-up when it +# RECOVERS. +# +# This is the shape a PIPELINE needs: a terminal JOB that `needs:` every other +# job in the workflow, so the alert fires on a failure anywhere in the graph, +# including a mid-graph failure that only skips its dependents rather than +# failing them. The release-train workflows (freeze, unfreeze, sync, release-pr) +# do not need a job, because they own a single job and can end with a step, so +# they call the `notify-pipeline-status` composite directly. Both run the same +# code — this file is a thin wrapper around that composite, and the alert's copy, +# its freeze scoping and its recovery detection all live there. # # A failed merge pipeline is a silent prod-risk event: main-triggered workflows -# cut releases and bake + deploy prod images. Callers add terminal jobs that -# `needs:` EVERY job in the workflow, so the alert fires on a failure in any job -# — including a mid-graph failure that only skips its dependents rather than -# failing them. +# cut releases and bake + deploy prod images. # # Two modes, selected by `status`: -# - failed: always posts a red alert. +# - failed: posts a red alert. # - recovered: posts a green "recovered" message ONLY when the previous # conclusive run of this workflow on this branch failed. On a normal green -# run (prior run already green) it no-ops, so the channel is not spammed on -# every merge. This is derived from the GitHub API rather than cross-run -# state, so it is self-healing (no cache to go stale). +# run it no-ops, so the channel is not spammed on every merge. Derived from +# the GitHub API rather than cross-run state, so it is self-healing. # -# `freeze-scoped: true` scopes a STAGING caller to the release window. The policy -# this implements: +# `freeze-scoped: true` scopes a STAGING caller to the release window: # -# failure on main -> alert -# failure on staging, freeze OPEN -> alert (staging is the release candidate) -# failure on staging, freeze NOT open -> nothing (ordinary integration-branch red) +# anything on main -> alert +# staging, freeze window OPEN -> alert +# staging, freeze window NOT open -> NOTHING, in either mode # -# Mid-week staging is the integration branch, and paging the channel for it is -# what teaches people to ignore the channel. `outside-freeze: notice` posts a -# muted grey message instead of nothing, for a caller that wants the trail; the -# default is silence. Either way the run is still red in the Actions tab, and -# pipeline-watchdog.yml still covers a staging branch that stops building at all. +# That last row means exactly what it says, and it is the fix for what this +# workflow used to do. On 2026-08-13 cowork-server's staging publish failed on +# the commit that FIXED a broken release candidate; the failure was correctly +# silent, and the re-run three and a half hours later posted a green "Recovered". +# The only thing the channel ever heard about a stalled rc stream was the good +# news. Posting only the green half is worse than posting neither. # -# Prod and freeze/unfreeze callers leave this alone: a failure there is always -# worth interrupting for. A workflow triggered on BOTH main and staging from one -# notify job cannot hardcode this — pass an expression instead, or a main failure -# gets silenced too: -# freeze-scoped: ${{ github.ref_name == 'staging' }} +# The run is still red in the Actions tab either way, and `pipeline-watchdog.yml` +# covers both a staging branch that stops building at all and one that STAYS red +# into the freeze window. # -# It posts its own message rather than the deploy-notification composite, whose -# copy is deploy-specific ("has failed deploying to ...") and reads wrong for -# non-deploy pipelines (freeze, CI, docs). It uses the same Slack bot token. +# Prod callers leave `freeze-scoped` alone: a failure there is always worth +# interrupting for. A workflow triggered on BOTH main and staging from one notify +# job cannot hardcode this — pass an expression instead, or a main failure gets +# silenced too: +# freeze-scoped: ${{ github.ref_name == 'staging' }} # -# ONE caller job covers both modes: a `uses:` job cannot branch on status, so -# the caller derives the aggregate outcome from `needs.*.result` and passes it -# in. Cancelled runs stay silent. +# ONE caller job covers both modes: a `uses:` job cannot branch on status, so the +# caller derives the aggregate outcome from `needs.*.result` and passes it in. +# Cancelled runs stay silent. # auth/.github/workflows/prod-build-deploy.yml # notify: # needs: [linter, run-unit-tests, build-auth, scan-auth, migrate, deploy, integration-tests] @@ -55,20 +60,16 @@ # status: ${{ contains(needs.*.result, 'failure') && 'failed' || 'recovered' }} # secrets: inherit # -# A failure-only caller (the freeze/unfreeze wrappers) keeps `if: failure()`, -# takes the default `status: failed`, and needs no `permissions:` block: the -# prior-run lookup only runs in recovered mode. -# # Requires (reach this workflow via `secrets: inherit` in the caller): # - secrets.SLACK_ENG_CHANNEL_ID (the engineering channel; org secret) # - secrets.GH_ACTIONS_SLACK_BOT_TOKEN (the Slack bot token; org secret) # The Slack bot must be a member of the channel SLACK_ENG_CHANNEL_ID resolves to. # -# Recovery mode also needs `actions: read` on the CALLER's notify job. The -# default workflow token carries contents + packages read only, and a called -# workflow can never hold more than its caller grants, so without that block the -# prior-run lookup is refused. It degrades to silence rather than failing, so a -# missing grant never turns a green pipeline red. +# Recovery mode also needs `actions: read` on the CALLER's notify job. The default +# workflow token carries contents + packages read only, and a called workflow can +# never hold more than its caller grants, so without that block the prior-run +# lookup is refused. It degrades to silence rather than failing, so a missing +# grant never turns a green pipeline red. name: Notify main-branch failure @@ -84,21 +85,37 @@ on: type: string default: failed freeze-scoped: - description: "Scale a 'failed' alert by release-freeze state (see `outside-freeze`). For staging pipelines, where a failure is only release-blocking once the freeze window is open." + description: "Silence this caller entirely while the staging freeze window is closed. For staging pipelines, where neither a failure nor its recovery is release-relevant mid-week." type: boolean default: false - outside-freeze: - description: "What a freeze-scoped failure does while staging is NOT frozen: 'silent' (post nothing) or 'notice' (muted, no panic styling)." + ruleset-name: + description: "The freeze ruleset to read. Must match what the repo's release-freeze caller sets, or this reads a name that does not exist and escalates every staging failure forever." type: string - default: silent + default: staging-freeze + force-post: + description: "Post a 'recovered' message with no prior failure to recover from. Only the smoke-test dispatch sets this; a real pipeline leaves it off, or every green run reports a recovery." + type: boolean + default: false runs-on: description: "Runner label for the notify job" type: string - default: mdb-dev + # GitHub-hosted on purpose. This job posts a Slack message and needs no + # cluster, no cloud and no self-hosted anything, and an alert that runs on + # the self-hosted fleet cannot report that the self-hosted fleet is down — + # the one outage where every pipeline goes red at once is the one where + # this would have nothing to run on. + default: ubuntu-latest # Manual smoke test: run this workflow from the Actions tab to post a sample - # message to the eng channel. `status: recovered` always posts here (the - # prior-run check is skipped on a manual dispatch), so both styles are - # testable on demand. + # message to the eng channel. `force-post` defaults ON here, so `status: + # recovered` posts without a prior failure and both styles are testable on + # demand. + # + # This used to be inferred from `github.event_name == 'workflow_dispatch'`, which + # was wrong: a called workflow inherits the CALLER's context, so that condition + # was also true when somebody manually ran a release-train wrapper, and every one + # of those declares `workflow_dispatch`. A successful manual "Staging Freeze" + # therefore posted a green Recovered for a failure that never happened. An input + # can tell the two apart; an event name cannot. workflow_dispatch: inputs: env-name: @@ -110,6 +127,10 @@ on: type: choice options: [failed, recovered] default: failed + force-post: + description: "Post even without a prior failure. On by default: this is the smoke test." + type: boolean + default: true runs-on: description: "Runner label" type: string @@ -123,232 +144,38 @@ jobs: notify: runs-on: ${{ inputs.runs-on }} steps: - # For the recovery message on a real pipeline run, only continue if the - # PREVIOUS conclusive run of this same workflow on this same branch failed. - # A manual dispatch skips the check and always posts (smoke test). - # - # The discriminator is the dispatch event, not `workflow_call`: a called - # workflow inherits the CALLER's `github` context, so `event_name` here is - # the caller's trigger (`push`, `workflow_run`, ...) and never - # `workflow_call`. - - name: Check whether the previous run was failing - id: prev - if: inputs.status == 'recovered' && github.event_name != 'workflow_dispatch' - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - WORKFLOW: ${{ github.workflow }} - WORKFLOW_REF: ${{ github.workflow_ref }} - BRANCH: ${{ github.ref_name }} - RUN_ID: ${{ github.run_id }} - RUN_ATTEMPT: ${{ github.run_attempt }} - run: | - set -uo pipefail - - # A RE-RUN is the most common way a failure gets fixed, and it was the - # one case this could never report. Attempts share a run id, so the - # failing attempt is the very run the history lookup below excludes as - # "the current one" — it would then find some older, green run and - # conclude nothing had broken. Check the previous attempt first. - if [ "${RUN_ATTEMPT:-1}" -gt 1 ]; then - PREV_ATTEMPT=$((RUN_ATTEMPT - 1)) - if LAST=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}/attempts/${PREV_ATTEMPT}" \ - --jq '.conclusion // ""' 2>&1); then - case "${LAST}" in - failure|timed_out|startup_failure) - echo "should_post=true" >> "$GITHUB_OUTPUT" - echo "prev_conclusion=${LAST}" >> "$GITHUB_OUTPUT" - echo "Attempt ${PREV_ATTEMPT} concluded '${LAST}', this attempt recovered it." - exit 0 - ;; - *) - echo "should_post=false" >> "$GITHUB_OUTPUT" - echo "Attempt ${PREV_ATTEMPT} concluded '${LAST:-unknown}', not a recovery." - exit 0 - ;; - esac - fi - echo "Could not read attempt ${PREV_ATTEMPT}, falling back to run history: ${LAST}" - fi - - # Match the workflow FILE rather than its display name, which a - # `run-name:` override changes; fall back to the name if the ref does - # not resolve to a path inside this repo. - WF_PATH="${WORKFLOW_REF%%@*}" - export WF_PATH="${WF_PATH#"${REPO}/"}" - - # Most recent CONCLUSIVE run on this branch, excluding the current one: - # cancelled and skipped runs are not evidence either way. - if ! PREV=$(gh api "repos/${REPO}/actions/runs?branch=${BRANCH}&status=completed&per_page=50" \ - --jq '[ .workflow_runs[] - | select(.path == env.WF_PATH or .name == env.WORKFLOW) - | select((.id|tostring) != env.RUN_ID) - | select(.conclusion == "success" or .conclusion == "failure" - or .conclusion == "timed_out" or .conclusion == "startup_failure") - ][0].conclusion // ""' 2>&1); then - # Almost always a missing `actions: read` grant on the caller job. - # Stay quiet rather than fail: a notify job must never redden a green run. - echo "Could not read run history, staying quiet: ${PREV}" - echo "should_post=false" >> "$GITHUB_OUTPUT" - exit 0 - fi - - echo "prev_conclusion=${PREV}" >> "$GITHUB_OUTPUT" - case "${PREV}" in - success|"") - echo "should_post=false" >> "$GITHUB_OUTPUT" - echo "Previous run concluded '${PREV:-none}', not a recovery, staying quiet." - ;; - *) - echo "should_post=true" >> "$GITHUB_OUTPUT" - echo "Previous run concluded '${PREV}', posting recovery." - ;; - esac - - # A staging pipeline failure means different things depending on where the - # release train is. Once the freeze window is open staging IS the release - # candidate, so a red pipeline blocks the release and is worth interrupting - # the channel for. Mid-week it is the integration branch and the same red - # is routine — panic styling there just teaches people to scroll past it. - # - # The freeze signal is the `staging-freeze` repository ruleset that the - # freeze/unfreeze workflows toggle between `active` and `disabled`, not an - # inference from workflow-run history: a freeze that skipped itself because - # staging had nothing unreleased still concludes `success`, and history - # cannot tell that apart from a real freeze. - # - # Reading rulesets needs admin, which the default workflow token does not - # carry, so this reuses the App that owns the toggle. Every failure path - # below falls through to `frozen=true`: a notify job that cannot establish - # the freeze state must escalate rather than silently downgrade a real - # release-blocking failure. + - name: Check out the shared alert implementation + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + repository: mindsdb/github-actions + # The commit of THIS reusable, not of main. A consumer that pins this + # workflow to a sha gets that sha's composite and scripts too, which is + # what makes the pin mean anything. + ref: ${{ job.workflow_sha }} + path: .ci-shared + persist-credentials: false + + # Minted here rather than inside the composite because a composite action's + # steps cannot use `continue-on-error`, and a repo with no release-train App + # must degrade to "assume frozen" rather than fail the notify job. - name: Mint release-train App token id: freeze-token - if: inputs.freeze-scoped && inputs.status != 'recovered' + if: inputs.freeze-scoped continue-on-error: true uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 with: client-id: ${{ vars.RELEASE_APP_CLIENT_ID }} private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} - - name: Check release-freeze state - id: freeze - if: inputs.freeze-scoped && inputs.status != 'recovered' - env: - GH_TOKEN: ${{ steps.freeze-token.outputs.token }} - REPO: ${{ github.repository }} - RULESET_NAME: staging-freeze - OUTSIDE_FREEZE: ${{ inputs.outside-freeze }} - run: | - set -uo pipefail - - escalate() { - echo "::warning::$1 Treating staging as frozen so this failure still escalates." - echo "frozen=true" >> "$GITHUB_OUTPUT" - exit 0 - } - - [ -n "${GH_TOKEN:-}" ] || escalate "Could not mint the release-train App token." - - ENFORCEMENT=$(gh api "repos/${REPO}/rulesets" \ - --jq ".[] | select(.name == \"${RULESET_NAME}\") | .enforcement" 2>&1) \ - || escalate "Could not read rulesets: ${ENFORCEMENT}." - [ -n "${ENFORCEMENT}" ] || escalate "Ruleset '${RULESET_NAME}' not found." - - if [ "${ENFORCEMENT}" = "active" ]; then - echo "frozen=true" >> "$GITHUB_OUTPUT" - echo "Ruleset '${RULESET_NAME}' is active — staging is frozen, escalating." - else - echo "frozen=false" >> "$GITHUB_OUTPUT" - echo "Ruleset '${RULESET_NAME}' is '${ENFORCEMENT}' — staging is not frozen, downgrading to '${OUTSIDE_FREEZE}'." - fi - - # Collapse the three outcomes (recovered / failed / failed-outside-freeze) - # into flat strings. Deciding this in shell keeps the Slack payload from - # growing a second and third nested ternary per field, which is where this - # kind of template stops being reviewable. - - name: Compose message - id: msg - env: - STATUS: ${{ inputs.status }} - FREEZE_SCOPED: ${{ inputs.freeze-scoped }} - FROZEN: ${{ steps.freeze.outputs.frozen }} - OUTSIDE_FREEZE: ${{ inputs.outside-freeze }} - run: | - set -euo pipefail - POST=true - if [ "${STATUS}" = "recovered" ]; then - LEVEL=recovered; COLOR='#00C851'; ICON=':white_check_mark:'; VERB=recovered - PREFIX=Recovered; NOTE='' - elif [ "${FREEZE_SCOPED}" = "true" ] && [ "${FROZEN}" = "false" ]; then - LEVEL=notice; COLOR='#B0B4BA'; ICON=':warning:'; VERB=failed - PREFIX=Failed; NOTE=' (outside the release freeze, not release-blocking)' - if [ "${OUTSIDE_FREEZE}" = "silent" ]; then POST=false; fi - else - LEVEL=alert; COLOR='#FF4444'; ICON=':rotating_light:'; VERB=failed - PREFIX=FAILED; NOTE='' - fi - { - echo "post=${POST}" - echo "level=${LEVEL}" - echo "color=${COLOR}" - echo "icon=${ICON}" - echo "verb=${VERB}" - echo "prefix=${PREFIX}" - echo "note=${NOTE}" - } >> "$GITHUB_OUTPUT" - echo "level=${LEVEL} post=${POST}" - - - name: Notify Slack - # Failure mode always posts; on a real pipeline, recovery posts only after - # a failing run; a manual dispatch always posts (smoke test). - # - # The payload carries a top-level `text` as well as the blocks. Slack - # renders the blocks, but `text` is what a push notification, a screen - # reader, and a notification-list preview show, so without it the alert - # arrives on a phone as an empty message — which is the one context where - # a pipeline failure most needs to be readable. - # `steps.msg.outputs.post` carries the one extra veto: a freeze-scoped - # failure outside the freeze window when the caller asked for silence. - if: steps.msg.outputs.post == 'true' && (inputs.status != 'recovered' || github.event_name == 'workflow_dispatch' || steps.prev.outputs.should_post == 'true') - uses: slackapi/slack-github-action@v1.26.0 + - name: Notify + uses: ./.ci-shared/notify-pipeline-status with: - channel-id: ${{ secrets.SLACK_ENG_CHANNEL_ID }} - payload: | - { - "text": "${{ steps.msg.outputs.prefix }}: ${{ github.event.repository.name }} ${{ inputs.env-name }} pipeline on ${{ github.ref_name }}${{ steps.msg.outputs.note }}", - "attachments": [ - { - "color": "${{ steps.msg.outputs.color }}", - "blocks": [ - { - "type": "section", - "text": { - "type": "mrkdwn", - "text": "${{ steps.msg.outputs.icon }} *<${{ github.server_url }}/${{ github.repository }}|${{ github.event.repository.name }}>* — *${{ inputs.env-name }}* pipeline ${{ steps.msg.outputs.verb }} on `${{ github.ref_name }}`${{ steps.msg.outputs.note }}" - }, - "fields": [ - { - "type": "mrkdwn", - "text": "*Workflow*\n<${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}|${{ github.workflow }}>" - }, - { - "type": "mrkdwn", - "text": "*${{ inputs.status == 'recovered' && 'Fixed by' || 'Triggered by' }}*\n${{ github.triggering_actor }}" - }, - { - "type": "mrkdwn", - "text": "*Commit*\n<${{ github.server_url }}/${{ github.repository }}/commit/${{ github.sha }}|${{ github.sha }}>" - }, - { - "type": "mrkdwn", - "text": "*Branch*\n<${{ github.server_url }}/${{ github.repository }}/tree/${{ github.ref_name }}|${{ github.ref_name }}>" - } - ] - } - ] - } - ] - } - env: - SLACK_BOT_TOKEN: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} + env-name: ${{ inputs.env-name }} + status: ${{ inputs.status }} + freeze-scoped: ${{ inputs.freeze-scoped }} + ruleset-name: ${{ inputs.ruleset-name }} + freeze-token: ${{ steps.freeze-token.outputs.token }} + github-token: ${{ github.token }} + force-post: ${{ inputs.force-post }} + slack-channel-id: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + slack-bot-token: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} diff --git a/.github/workflows/notify-startup-failure.yml b/.github/workflows/notify-startup-failure.yml index d9cc65b..b15a337 100644 --- a/.github/workflows/notify-startup-failure.yml +++ b/.github/workflows/notify-startup-failure.yml @@ -1,5 +1,23 @@ -# Reusable workflow: alert the engineering channel when a pipeline on a -# protected branch concluded WITHOUT EVER STARTING. +# Reusable workflow: watch the deploy branches from OUTSIDE their pipelines. +# +# Two sweeps, both on the caller's schedule, both reporting things the pipeline's +# own terminal notify job structurally cannot: +# +# startup-failures a run concluded without ever starting, so there was no job +# to put an alert in. +# red-branches the newest run of a pipeline is red and nobody is being +# told: because the alert was deliberately silenced, because +# it was sent once and missed, or because the re-run did not +# re-notify. +# +# They are separate jobs with separate messages because they are separate +# findings with separate fixes, and one incident producing two alerts is how an +# alert channel gets muted. `startup_failure` is reported only by the first, and +# excluded from the second. +# +# --- +# +# SWEEP 1 — a pipeline that concluded WITHOUT EVER STARTING. # # Why this exists as a separate mechanism from notify-main-failure.yml, rather # than another mode of it. @@ -65,8 +83,49 @@ # repository's DEFAULT branch. A watchdog caller merged to `staging` and no # further is inert. `workflow_dispatch` is there so it can be proven before it # reaches `main`. +# +# --- +# +# SWEEP 2 — a deploy branch that is STILL red. +# +# The in-run notify job reports a failure once, at the moment it happens. Three +# cases that leaves uncovered, all of which have happened: +# +# 1. Nobody was going to be told. A staging failure outside the release-freeze +# window is deliberately silent (see notify-main-failure.yml). The branch is +# still red when Friday's freeze turns it into the release candidate, and at +# that moment nothing says so. +# 2. It was reported once and stayed broken. One message is easy to miss and +# there is no second one. +# 3. The re-run did not re-notify. "Re-run failed jobs" re-runs the failed job +# and everything downstream, so the terminal notify job fires again. The +# per-job "Re-run this job" button re-runs that job alone, so a run can go +# red to green with notify never running twice. No hook fixes that from +# inside the run. +# +# It is a BACKSTOP, not an echo. A finding must be at least `min-age-minutes` +# old, which turns the message from "this failed" (which the pipeline already +# said) into "this is still failing and nobody has touched it". The exception is +# the freeze window opening, where the finding is old by definition and the news +# is the window. +# +# On `staging` it only speaks while the freeze is on, and the window OPENING is +# itself a trigger, so a branch that broke on Tuesday is reported on Friday. That +# is keyed on a successful run of the "Staging Freeze" workflow rather than on a +# clock, so moving the freeze moves the alerting with it and neither has to know +# about the other. The freeze runs are read through the workflow's own runs +# endpoint, because a `schedule` trigger is attributed to the DEFAULT branch and +# they are not in a `staging` listing at all. +# +# Selection logic lives in `scripts/branch_health.py` with unit tests, because it +# is date arithmetic plus a two-way trigger and that is not something to hold in +# a jq expression. -name: Notify pipeline startup failure +# The FILE NAME stays `notify-startup-failure.yml` even though it now carries +# both sweeps: every caller references it by path, so renaming it would break +# seven repos at once for a cosmetic gain. + +name: Notify deploy-branch health on: workflow_call: @@ -76,19 +135,63 @@ on: type: string default: "main staging" lookback-minutes: - description: "Only alert on a startup failure that began this recently" + description: "Only alert on a finding that began this recently" type: number default: 90 + red-branch-sweep: + description: "Also report a deploy branch whose newest pipeline run is still red" + type: boolean + default: true + min-age-minutes: + description: "How long a failure must have stood before the red-branch sweep repeats it, so the sweep is a backstop rather than an echo of the alert the pipeline already sent" + type: number + default: 30 + workflows: + description: "Space-separated workflow paths the red-branch sweep may report on. Empty means every workflow with a run on these branches, which is wider than the set that opted into an in-run alert: the sweep reads run history, not notify wiring, so a red scheduled job or a red one-off check on a deploy branch counts too. Narrow it here if that is not wanted." + type: string + default: "" + staging-branch: + description: "Which of `branches` is the release candidate, and so is only reported while the freeze window is open" + type: string + default: staging + ruleset-name: + description: "The freeze ruleset to read. Must match what the repo's release-freeze caller sets." + type: string + default: staging-freeze runs-on: - description: "Runner label for the sweep job" + description: "Runner label for the sweep jobs" type: string - default: mdb-dev + # GitHub-hosted on purpose. These sweeps read the API and post to Slack; + # they need no cluster, no cloud and no self-hosted anything, and a + # watchdog that runs on the self-hosted fleet cannot report that the + # self-hosted fleet is down. + default: ubuntu-latest workflow_dispatch: inputs: branches: description: "Space-separated branches to sweep" type: string default: "main staging" + red-branch-sweep: + description: "Also report a deploy branch whose newest pipeline run is still red" + type: boolean + default: true + min-age-minutes: + description: "Age floor for the red-branch sweep (set 0 to report a failure that just happened)" + type: number + default: 30 + workflows: + description: "Workflow paths to report on (space separated); empty means all" + type: string + default: "" + staging-branch: + description: "Which branch is the release candidate" + type: string + default: staging + ruleset-name: + description: "The freeze ruleset to read" + type: string + default: staging-freeze lookback-minutes: description: "Lookback window in minutes (widen it to replay a past incident)" type: number @@ -261,3 +364,195 @@ jobs: exit 1 fi echo "Alert posted." >> "$GITHUB_STEP_SUMMARY" + + # SWEEP 2 (see the header): deploy branches whose newest pipeline run is still + # red. Independent of the sweep above rather than a stage of it — one sweep + # failing must not hide the other's findings, and they report different things. + red-branches: + if: inputs.red-branch-sweep + runs-on: ${{ inputs.runs-on }} + steps: + - name: Check out the shared sweep and freeze contract + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + repository: mindsdb/github-actions + # The commit of THIS reusable, so a branch under review runs its own + # version rather than silently testing main's. + ref: ${{ job.workflow_sha }} + path: .ci-shared + persist-credentials: false + + # Reading rulesets needs admin, which the default workflow token does not + # carry, so this reuses the App that owns the freeze toggle. A repo that is + # not on the release train has no App and no ruleset; the read escalates to + # frozen, which for this sweep means staging is swept like any other branch. + - name: Mint release-train App token + id: freeze-token + continue-on-error: true + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + client-id: ${{ vars.RELEASE_APP_CLIENT_ID }} + private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} + + - name: Check release-freeze state + id: freeze + env: + GH_TOKEN: ${{ steps.freeze-token.outputs.token }} + REPO: ${{ github.repository }} + RULESET_NAME: ${{ inputs.ruleset-name }} + run: | + set -uo pipefail + if [ -z "${GH_TOKEN:-}" ]; then + echo "::warning::Could not mint the release-train App token. Treating the branch as frozen so a red staging still surfaces." + echo "frozen=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + python3 .ci-shared/scripts/freeze_state.py read \ + --repo "${REPO}" --ruleset-name "${RULESET_NAME}" --on-error escalate + + - name: Find deploy branches that are still red + id: find + env: + GH_TOKEN: ${{ github.token }} + REPO: ${{ github.repository }} + BRANCHES: ${{ inputs.branches }} + STAGING: ${{ inputs.staging-branch }} + LOOKBACK: ${{ inputs.lookback-minutes }} + MIN_AGE: ${{ inputs.min-age-minutes }} + FROZEN: ${{ steps.freeze.outputs.frozen }} + WORKFLOWS: ${{ inputs.workflows }} + WORKFLOW_REF: ${{ github.workflow_ref }} + run: | + set -uo pipefail + + # The sweep's own workflow, so it never reports itself. Derived rather + # than asked of every caller: `github.workflow_ref` is the CALLER's + # top-level workflow, which is exactly the watchdog. A watchdog run that + # went red would otherwise be the newest red run of a workflow on `main` + # and get alerted on at the next tick, reading as a pipeline failure. + SELF_PATH="${WORKFLOW_REF%%@*}" + SELF_PATH="${SELF_PATH#"${REPO}/"}" + + python3 .ci-shared/scripts/branch_health.py \ + --repo "${REPO}" \ + --branches "${BRANCHES}" \ + --staging-branch "${STAGING}" \ + --lookback-minutes "${LOOKBACK}" \ + --min-age-minutes "${MIN_AGE}" \ + --frozen "${FROZEN}" \ + --workflows "${WORKFLOWS}" \ + --self-path "${SELF_PATH}" \ + --out red-branches.json + + # A separate step because it reads `find`'s outputs, and a step cannot read + # its own. `always()` so a failed lookup still explains itself. + - name: Summarise the sweep + if: always() && steps.find.conclusion != 'skipped' + env: + BRANCHES: ${{ inputs.branches }} + LOOKBACK: ${{ inputs.lookback-minutes }} + MIN_AGE: ${{ inputs.min-age-minutes }} + FROZEN: ${{ steps.freeze.outputs.frozen }} + WORKFLOWS: ${{ inputs.workflows }} + COUNT: ${{ steps.find.outputs.count }} + UNREADABLE: ${{ steps.find.outputs.unreadable }} + run: | + set -uo pipefail + { + echo "### Red-branch sweep" + echo + echo "Branches: \`${BRANCHES}\` · window: last ${LOOKBACK} min · age floor: ${MIN_AGE} min · staging frozen: ${FROZEN:-unknown}" + echo "Scope: \`${WORKFLOWS:-every workflow on the branch}\`" + echo + if [ -n "${UNREADABLE}" ]; then + # Never an all-clear for a branch this could not read. The + # "nothing to report" line below used to print regardless, so a 403 + # read as "everything is fine". + echo "**Could not read run history for: \`${UNREADABLE}\`.** Those branches were not checked," + echo "so this is not an all-clear for them. Usually a missing \`actions: read\` grant on the" + echo "calling job." + echo + fi + if [ ! -s red-branches.json ]; then + echo "The sweep did not finish, so nothing was checked." + elif [ "${COUNT:-0}" = "0" ]; then + echo "No deploy branch is sitting red. Nothing to report." + else + echo "| Workflow | Branch | Commit | Conclusion | Why now | Run |" + echo "| --- | --- | --- | --- | --- | --- |" + jq -r '.[] | "| `\(.path)` | `\(.branch)` | `\(.head_sha[0:8])` | \(.conclusion) | \(.reason) | [\(.id)](\(.html_url)) |"' red-branches.json + fi + } >> "$GITHUB_STEP_SUMMARY" + + - name: Post the alert + if: steps.find.outputs.count != '0' && !inputs.dry-run + env: + SLACK_BOT_TOKEN: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} + CHANNEL: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + REPO: ${{ github.repository }} + REPO_NAME: ${{ github.event.repository.name }} + SERVER: ${{ github.server_url }} + SWEEP_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -uo pipefail + + # The lead line names WHY this arrived, because the two reasons want + # different reactions: "still red" means the alert you already saw was + # not acted on, and "freeze-opened" means a branch you were right to + # ignore mid-week just became the release candidate. + jq -n \ + --slurpfile findings red-branches.json \ + --arg repo "$REPO" --arg repo_name "$REPO_NAME" \ + --arg server "$SERVER" --arg channel "$CHANNEL" --arg sweep "$SWEEP_URL" ' + # Keyed on the finding that actually carries the reason, not on any() + # across every branch. `reason` is per-branch, so an any() over a + # mixed set announced a red `main` as "the release freeze has opened + # and staging is red". + ([$findings[0][] | select(.reason == "freeze-opened")] | first) as $froze | + { + channel: $channel, + text: (if $froze + then "\($repo_name) — the release freeze opened on a red \($froze.branch)" + else "\($repo_name) — a deploy branch is still red" end), + attachments: [{ + color: "#FF4444", + blocks: ( + [{ + type: "section", + text: { + type: "mrkdwn", + text: (if $froze + then ":snowflake: *<\($server)/\($repo)|\($repo_name)>* — the release freeze has opened and `\($froze.branch)` is *red*. It is the release candidate now, so this blocks the release until it is fixed." + else ":rotating_light: *<\($server)/\($repo)|\($repo_name)>* — \($findings[0] | length) pipeline(s) *still failing*. The newest run is red and nobody has fixed it." end) + } + }] + + ( $findings[0] | map({ + type: "section", + fields: [ + { type: "mrkdwn", text: "*Workflow*\n<\($server)/\($repo)/actions/runs/\(.id)|\(.path)>" }, + { type: "mrkdwn", text: "*Branch*\n<\($server)/\($repo)/tree/\(.branch)|\(.branch)>" }, + { type: "mrkdwn", text: "*Commit*\n<\($server)/\($repo)/commit/\(.head_sha)|\(.head_sha[0:8])>" }, + { type: "mrkdwn", text: "*Last touched by*\n\(.actor)" } + ] + }) ) + + [{ + type: "context", + elements: [{ + type: "mrkdwn", + text: "Re-run the failed jobs, or fix forward. Note that re-running a SINGLE job does not re-fire the pipeline notify job, which is one of the reasons this sweep exists. <\($sweep)|Sweep run>" + }] + }] + ) + }] + }' > payload.json + + RESPONSE=$(curl -sS -X POST https://slack.com/api/chat.postMessage \ + -H "Authorization: Bearer ${SLACK_BOT_TOKEN}" \ + -H 'Content-type: application/json; charset=utf-8' \ + --data @payload.json) + + if [ "$(jq -r '.ok' <<< "$RESPONSE")" != "true" ]; then + echo "Slack rejected the alert: $(jq -r '.error // "unknown"' <<< "$RESPONSE")" + exit 1 + fi + echo "Alert posted." >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/release-freeze.yml b/.github/workflows/release-freeze.yml index 33945b1..ae5e353 100644 --- a/.github/workflows/release-freeze.yml +++ b/.github/workflows/release-freeze.yml @@ -38,8 +38,20 @@ on: type: string default: staging-freeze -permissions: - contents: read +# No workflow-level `permissions:` block, deliberately, so this job inherits the +# calling job's grant. That is what makes the notify step's recovery lookup work: +# a block here would be the CEILING for this job's token as well as the caller's, +# so declaring `contents: read` alone would cap the token to `contents: read` and +# the lookup would be refused no matter what the caller granted. Declaring +# `actions: read` instead is not an option either — every caller that has not yet +# granted it would fail to LOAD, and these wrappers reach a repo's default branch +# only at the next weekly release, so the two can never merge in step. Inheriting +# gives both: the grant where a caller has it, and a refused lookup that logs why +# and stays quiet where it does not. +# +# The cost is real and worth naming: `scripts/workflow_graph.py` check 2 compares +# what a callee DECLARES against what its callers grant, so a job that declares +# nothing is invisible to it. That check cannot help here. jobs: freeze-staging: @@ -63,6 +75,18 @@ jobs: fetch-depth: 0 persist-credentials: false + # The ruleset read-modify-write lives in `scripts/freeze_state.py`, which is + # also what the alerting workflows use to READ the freeze state. One file + # owns the contract, so a repo that renames its ruleset cannot end up frozen + # by one name and alerted on another. + - name: Check out the shared release-freeze contract + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + repository: mindsdb/github-actions + ref: ${{ job.workflow_sha }} + path: .ci-shared + persist-credentials: false + - name: Check for unreleased changes id: check env: @@ -98,23 +122,38 @@ jobs: ENFORCEMENT: active run: | set -euo pipefail - RID=$(gh api "repos/${REPO}/rulesets" \ - --jq ".[] | select(.name == \"${RULESET_NAME}\") | .id") - if [ -z "$RID" ]; then - echo "::error::Ruleset '${RULESET_NAME}' not found in ${REPO} — provisioning has drifted." - exit 1 - fi - # GET the full ruleset, flip only `enforcement`, PUT the whole body back. - # A partial PUT is not guaranteed to preserve omitted fields. The body is - # written to a temp file (never echoed) so ruleset internals don't leak - # into logs — three of these repos are public. - BODY="${RUNNER_TEMP}/ruleset-${RID}.json" - gh api "repos/${REPO}/rulesets/${RID}" \ - --jq "{name, target, enforcement: \"${ENFORCEMENT}\", bypass_actors, conditions, rules}" > "$BODY" - gh api --method PUT "repos/${REPO}/rulesets/${RID}" --input "$BODY" > /dev/null - echo "Ruleset '${RULESET_NAME}' (#${RID}) enforcement set to ${ENFORCEMENT} — ${STAGING} is FROZEN." + # Fails loudly on purpose, unlike the alerting path's read: a freeze + # that could not be applied has to stop the release train rather than + # let the window appear to open. + python3 .ci-shared/scripts/freeze_state.py set \ + --repo "${REPO}" \ + --ruleset-name "${RULESET_NAME}" \ + --enforcement "${ENFORCEMENT}" \ + --body-path "${RUNNER_TEMP}/ruleset.json" + echo "${STAGING} is FROZEN." { echo "## Staging branch FROZEN" echo "" echo "The \`${STAGING}\` branch is **frozen** via ruleset \`${RULESET_NAME}\`." } >> "$GITHUB_STEP_SUMMARY" + + # The alert lives here rather than in each repo's wrapper, which is what + # made every wrapper `if: failure()` and therefore incapable of ever saying + # a failure had been FIXED. Ending the job with this step covers both + # directions in one place for all seven consumers. + # `continue-on-error` because the alert must never decide the job's fate. It + # runs under `always()` on green runs too now, and the Slack action calls + # `core.setFailed` on any Slack-side error, so without this a rotated bot + # token or a bot removed from the channel would report a successful run as a + # failure — and for the freeze that also skips the release-PR refresh, which + # gates on this run's conclusion. + - name: Notify the engineering channel + if: ${{ always() && job.status != 'cancelled' }} + continue-on-error: true + uses: ./.ci-shared/notify-pipeline-status + with: + env-name: "staging freeze" + status: ${{ job.status == 'failure' && 'failed' || 'recovered' }} + github-token: ${{ github.token }} + slack-channel-id: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + slack-bot-token: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml index ddecfa4..2f900dd 100644 --- a/.github/workflows/release-pr.yml +++ b/.github/workflows/release-pr.yml @@ -1,11 +1,30 @@ -# Reusable workflow: open (or update) the weekly staging -> main release PR. +# Reusable workflow: keep the staging -> main release PR open and current. # -# Chains off a successful "Staging Freeze" run via the caller's workflow_run -# trigger. Idempotent: if an open staging -> main PR already exists it is left -# untouched. Skips entirely when staging has no unreleased changes. +# It used to run once a week, at freeze time, and the release queue was invisible +# until then: the only way to know what Friday would ship was to diff two +# branches by hand. So the caller now also fires it after each staging pipeline, +# and this workflow UPDATES an open PR rather than skipping it. The commit list +# and contributor list are rewritten every time, so the PR is a live view of what +# is queued from the first merge after an unfreeze. # -# The ahead-check is kept even though the caller guards on freeze success — a -# manual workflow_dispatch can invoke this workflow directly. +# **It opens as a DRAFT and becomes ready at freeze time.** An always-open +# staging -> main PR is a merge button sitting next to production all week, and +# the whole point of the freeze window is that staging is only a release +# candidate inside it. Draft is what makes the rolling PR safe: it cannot be +# merged by accident, GitHub greys it out, and the transition to ready is a +# visible event that says the window is open. The freeze state is read from the +# same `scripts/freeze_state.py` the freeze workflow writes, so the two cannot +# disagree about when that is. +# +# Idempotent in both directions: it never opens a second PR, and re-running it +# against an unchanged branch rewrites the same body. The find-then-create is not +# atomic though, so the CALLER carries a `concurrency: release-pr` group. Without +# it the freeze and a staging merge in the same minute produce two runs, the loser's +# `gh pr create` takes a 422, and `set -euo pipefail` turns that into a red +# "release PR pipeline failed" alert for a PR that exists and is fine. +# +# The ahead-check is kept even though the freeze caller guards on freeze success — +# a manual workflow_dispatch can invoke this workflow directly. # # The PR is created with the `mindsdb-release-train` App token (NOT the default # GITHUB_TOKEN) so that opening the PR triggers the normal CI checks — PRs @@ -31,9 +50,25 @@ on: description: "Target branch for the release PR" type: string default: main + ruleset-name: + description: "The freeze ruleset to read, to decide draft vs ready. Must match what the repo's release-freeze caller sets." + type: string + default: staging-freeze -permissions: - contents: read +# No workflow-level `permissions:` block, deliberately, so this job inherits the +# calling job's grant. That is what makes the notify step's recovery lookup work: +# a block here would be the CEILING for this job's token as well as the caller's, +# so declaring `contents: read` alone would cap the token to `contents: read` and +# the lookup would be refused no matter what the caller granted. Declaring +# `actions: read` instead is not an option either — every caller that has not yet +# granted it would fail to LOAD, and these wrappers reach a repo's default branch +# only at the next weekly release, so the two can never merge in step. Inheriting +# gives both: the grant where a caller has it, and a refused lookup that logs why +# and stays quiet where it does not. +# +# The cost is real and worth naming: `scripts/workflow_graph.py` check 2 compares +# what a callee DECLARES against what its callers grant, so a job that declares +# nothing is invisible to it. That check cannot help here. jobs: create-pr: @@ -51,6 +86,32 @@ jobs: fetch-depth: 0 persist-credentials: false + - name: Check out the shared release-freeze contract + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + repository: mindsdb/github-actions + ref: ${{ job.workflow_sha }} + path: .ci-shared + persist-credentials: false + + # Decides draft vs ready. Escalating to "frozen" on a lookup failure is the + # safe direction everywhere else in this system; here it is not, because it + # would mark the PR ready to merge on a branch nobody has validated. So this + # one reader fails closed the other way. + - name: Check release-freeze state + id: freeze + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + REPO: ${{ github.repository }} + RULESET_NAME: ${{ inputs.ruleset-name }} + run: | + set -uo pipefail + if ! python3 .ci-shared/scripts/freeze_state.py read \ + --repo "${REPO}" --ruleset-name "${RULESET_NAME}" --on-error fail; then + echo "::warning::Could not read the freeze state. Leaving the release PR as a draft." + echo "frozen=false" >> "$GITHUB_OUTPUT" + fi + - name: Check for unreleased changes id: check env: @@ -78,25 +139,47 @@ jobs: AHEAD: ${{ steps.check.outputs.ahead }} STAGING: ${{ inputs.staging-branch }} BASE: ${{ inputs.base-branch }} + FROZEN: ${{ steps.freeze.outputs.frozen }} run: | set -euo pipefail - # Skip if a staging -> main PR is already open. + COMMITS=$(git log --oneline --no-merges "origin/${BASE}..origin/${STAGING}") + CONTRIBUTORS=$(git log --format='%an' "origin/${BASE}..origin/${STAGING}" | sort -u | sed 's/^/- /') + + # Find the PR before writing about it. The body describes whether this is + # a draft, and only the PR itself knows that: an existing PR may already + # be ready for review, either because a human marked it or because it + # predates the draft behaviour, and nothing below ever re-drafts one. A + # state line derived from the freeze alone told four already-ready release + # PRs that they were "not ready to merge". EXISTING=$(gh pr list --repo "${REPO}" --base "${BASE}" --head "${STAGING}" \ --state open --json number --jq '.[0].number // empty') + + WAS_DRAFT=true if [ -n "$EXISTING" ]; then - echo "PR #${EXISTING} already exists — skipping creation." - echo "::notice title=Weekly release PR::https://github.com/${REPO}/pull/${EXISTING}" - exit 0 + WAS_DRAFT=$(gh pr view "$EXISTING" --repo "${REPO}" --json isDraft --jq '.isDraft') fi - COMMITS=$(git log --oneline --no-merges "origin/${BASE}..origin/${STAGING}") - CONTRIBUTORS=$(git log --format='%an' "origin/${BASE}..origin/${STAGING}" | sort -u | sed 's/^/- /') + # What the PR will be once this run is done: draft only while the window + # is closed AND it is not already ready. + if [ "${FROZEN}" = "true" ] || [ "${WAS_DRAFT}" != "true" ]; then + DRAFT_AFTER=false + else + DRAFT_AFTER=true + fi + + if [ "${DRAFT_AFTER}" = "true" ]; then + STATE_LINE="> Draft until the release freeze opens. \`${STAGING}\` is still the integration branch, so this list will keep growing and this PR is not ready to merge." + elif [ "${FROZEN}" = "true" ]; then + STATE_LINE="> The release freeze is **open**: \`${STAGING}\` is the release candidate and this PR is ready to merge once it has been validated." + else + STATE_LINE="> The release freeze is **closed**, so \`${STAGING}\` is still the integration branch and this list will keep growing. This PR is already marked ready for review and nothing re-drafts it, so treat it as mergeable only once the window opens." + fi BODY=$(cat < Created automatically at staging freeze time. Merge when ready to release. + ${STATE_LINE} + > + > Opened and kept current automatically. The commit list is rewritten on every + > staging merge, so it always reflects what would ship right now. EOF ) @@ -119,10 +205,56 @@ jobs: BODY="${BODY//$'\n' /$'\n'}" BODY="${BODY# }" - PR_URL=$(gh pr create \ - --repo "${REPO}" \ - --base "${BASE}" \ - --head "${STAGING}" \ - --title "Weekly release: ${STAGING} → ${BASE}" \ - --body "$BODY") - echo "::notice title=Weekly release PR::${PR_URL}" + TITLE="Release: ${STAGING} → ${BASE} (${AHEAD} commits)" + + if [ -z "$EXISTING" ]; then + # New PR. Draft unless the window is already open, so a rolling PR can + # never be merged during the week it is accumulating. + DRAFT=() + [ "${DRAFT_AFTER}" = "true" ] && DRAFT=(--draft) + PR_URL=$(gh pr create \ + --repo "${REPO}" \ + --base "${BASE}" \ + --head "${STAGING}" \ + --title "$TITLE" \ + --body "$BODY" \ + "${DRAFT[@]}") + echo "::notice title=Release PR::${PR_URL}" + exit 0 + fi + + gh pr edit "$EXISTING" --repo "${REPO}" --title "$TITLE" --body "$BODY" + + # Ready-for-review is the visible signal that the window opened. Only + # ever draft -> ready: re-drafting a PR someone deliberately marked ready + # would fight the human. + if [ "${FROZEN}" = "true" ] && [ "${WAS_DRAFT}" = "true" ]; then + gh pr ready "$EXISTING" --repo "${REPO}" + echo "Freeze is open — PR #${EXISTING} marked ready for review." + fi + echo "::notice title=Release PR::https://github.com/${REPO}/pull/${EXISTING}" + + # The alert lives here rather than in each repo's wrapper, which is what + # made every wrapper `if: failure()` and therefore incapable of ever saying + # a failure had been FIXED. Ending the job with this step covers both + # directions in one place for all seven consumers. + # `continue-on-error` because the alert must never decide the job's fate. It + # runs under `always()` on green runs too now, and the Slack action calls + # `core.setFailed` on any Slack-side error, so without this a rotated bot + # token or a bot removed from the channel would report a successful run as a + # failure — and for the freeze that also skips the release-PR refresh, which + # gates on this run's conclusion. + - name: Notify the engineering channel + if: ${{ always() && job.status != 'cancelled' }} + continue-on-error: true + uses: ./.ci-shared/notify-pipeline-status + with: + env-name: "release PR" + status: ${{ job.status == 'failure' && 'failed' || 'recovered' }} + # This workflow is reached by `workflow_run`, where GITHUB_REF is the + # DEFAULT branch rather than the branch whose pipeline finished, so the + # message would otherwise say the release PR failed on `main`. + branch: ${{ inputs.staging-branch }} + github-token: ${{ github.token }} + slack-channel-id: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + slack-bot-token: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} diff --git a/.github/workflows/release-unfreeze.yml b/.github/workflows/release-unfreeze.yml index b5edbb3..972a5ed 100644 --- a/.github/workflows/release-unfreeze.yml +++ b/.github/workflows/release-unfreeze.yml @@ -3,7 +3,9 @@ # # Unfreezing flips the pre-provisioned `staging-freeze` ruleset back to # `disabled`, reopening staging for normal development the moment the release -# ships — no fixed Monday-morning wait. +# ships — no fixed Monday-morning wait. It is also what turns staging's alerting +# back off: the alert path reads the same ruleset, so an unfrozen staging stops +# paging the channel without anything else being told. # # Order is load-bearing: unlock first, then push. If the sync-back push fails, # staging is left unlocked, which is the acceptable failure mode. @@ -18,7 +20,8 @@ # cowork-server/.github/workflows/staging-unfreeze.yml # The wrapper carries the merged-release-PR guard (head.ref == staging AND # head.repo.full_name == the repo, to reject fork PRs). Keep the name -# "Staging Unfreeze". +# "Staging Unfreeze". The wrapper carries NO notify job: this workflow ends with +# one, so a re-run that fixes a failed unfreeze reports itself. # # Requires: vars.RELEASE_APP_CLIENT_ID + secrets.RELEASE_APP_PRIVATE_KEY # (org-provisioned; secret reaches here via `secrets: inherit`). @@ -41,8 +44,20 @@ on: type: string default: staging-freeze -permissions: - contents: read +# No workflow-level `permissions:` block, deliberately, so this job inherits the +# calling job's grant. That is what makes the notify step's recovery lookup work: +# a block here would be the CEILING for this job's token as well as the caller's, +# so declaring `contents: read` alone would cap the token to `contents: read` and +# the lookup would be refused no matter what the caller granted. Declaring +# `actions: read` instead is not an option either — every caller that has not yet +# granted it would fail to LOAD, and these wrappers reach a repo's default branch +# only at the next weekly release, so the two can never merge in step. Inheriting +# gives both: the grant where a caller has it, and a refused lookup that logs why +# and stays quiet where it does not. +# +# The cost is real and worth naming: `scripts/workflow_graph.py` check 2 compares +# what a callee DECLARES against what its callers grant, so a job that declares +# nothing is invisible to it. That check cannot help here. jobs: unfreeze-staging: @@ -55,6 +70,23 @@ jobs: client-id: ${{ vars.RELEASE_APP_CLIENT_ID }} private-key: ${{ secrets.RELEASE_APP_PRIVATE_KEY }} + # The ruleset read-modify-write lives in `scripts/freeze_state.py`, which is + # also what the alerting workflows use to READ the freeze state. One file + # owns the contract, so unfreezing and "staging alerts are off again" can + # never disagree about which ruleset they mean. + # + # This has to precede the unlock, because the unlock runs that script. It is + # therefore the one step whose failure leaves staging FROZEN, which is the + # bad direction — so it is also the only one, and the credentialed checkout + # that used to sit here moved below the unlock where it belongs. + - name: Check out the shared release-freeze contract + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + repository: mindsdb/github-actions + ref: ${{ job.workflow_sha }} + path: .ci-shared + persist-credentials: false + - name: Unfreeze staging (disable ruleset) env: GH_TOKEN: ${{ steps.app-token.outputs.token }} @@ -64,50 +96,49 @@ jobs: ENFORCEMENT: disabled run: | set -euo pipefail - RID=$(gh api "repos/${REPO}/rulesets" \ - --jq ".[] | select(.name == \"${RULESET_NAME}\") | .id") - if [ -z "$RID" ]; then - echo "::error::Ruleset '${RULESET_NAME}' not found in ${REPO} — provisioning has drifted." - exit 1 - fi - # GET the full ruleset, flip only `enforcement`, PUT the whole body back - # (partial PUT is not guaranteed to preserve omitted fields). Body goes to - # a temp file, never echoed, so ruleset internals stay out of public logs. - BODY="${RUNNER_TEMP}/ruleset-${RID}.json" - gh api "repos/${REPO}/rulesets/${RID}" \ - --jq "{name, target, enforcement: \"${ENFORCEMENT}\", bypass_actors, conditions, rules}" > "$BODY" - gh api --method PUT "repos/${REPO}/rulesets/${RID}" --input "$BODY" > /dev/null + python3 .ci-shared/scripts/freeze_state.py set \ + --repo "${REPO}" \ + --ruleset-name "${RULESET_NAME}" \ + --enforcement "${ENFORCEMENT}" \ + --body-path "${RUNNER_TEMP}/ruleset.json" PR_NUMBER='${{ github.event.pull_request.number }}' - echo "Ruleset '${RULESET_NAME}' (#${RID}) enforcement set to ${ENFORCEMENT} — ${STAGING} is UNFROZEN." + echo "${STAGING} is UNFROZEN." { echo "## Staging branch UNFROZEN" echo "" echo "The \`${STAGING}\` branch is **unfrozen** via ruleset \`${RULESET_NAME}\` (triggered by PR #${PR_NUMBER:-manual})." } >> "$GITHUB_STEP_SUMMARY" - # Only the sync-back checkout carries credentials — it is the one step that - # pushes. + # Order is load-bearing and this is the whole reason for it: unlock first, + # then push. If anything from here down fails, staging is left UNLOCKED, + # which is the acceptable direction. Checking out is not a push, but it can + # still fail, so it sits after the unlock rather than before it. + # + # `path:` keeps it out of the workspace root, so it cannot clean the shared + # checkout above back out and the order is free rather than a constraint. # - # Runs on a manual dispatch too, which it did not used to. The old guard was - # `github.event_name == 'pull_request'`, reasoned as "no PR merged, nothing - # to sync", and that reasoning is wrong in the one case that matters: a - # dispatch is what you reach for when the PR-triggered run could not run. - # On 2026-07-27 auth's PR-triggered unfreeze died as a `startup_failure` - # (an invalid caller, so zero jobs), the operator recovered with a dispatch, - # and it unfroze staging while silently skipping the sync — leaving main - # squash-merged into a `main` that `staging` did not contain, which had to be - # merged by hand an hour later. The escape hatch was strictly less capable - # than the path it exists to replace. + # The sync step runs on a manual dispatch too, which it did not used to. The + # old guard was `github.event_name == 'pull_request'`, reasoned as "no PR + # merged, nothing to sync", and that reasoning is wrong in the one case that + # matters: a dispatch is what you reach for when the PR-triggered run could + # not run. On 2026-07-27 auth's PR-triggered unfreeze died as a + # `startup_failure` (an invalid caller, so zero jobs), the operator recovered + # with a dispatch, and it unfroze staging while silently skipping the sync — + # leaving main squash-merged into a `main` that `staging` did not contain, + # which had to be merged by hand an hour later. The escape hatch was strictly + # less capable than the path it exists to replace. # - # Nothing needs the event to decide this: the sync step below already exits - # 0 when `staging` is an ancestor of `main`, so a dispatch with genuinely + # Nothing needs the event to decide this: the sync step below already exits 0 + # when `staging` is an ancestor of `main`, so a dispatch with genuinely # nothing to sync is a logged no-op rather than a wasted push. - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 with: fetch-depth: 0 token: ${{ steps.app-token.outputs.token }} + path: repo - name: Sync main back into staging + working-directory: repo env: GH_TOKEN: ${{ steps.app-token.outputs.token }} APP_SLUG: ${{ steps.app-token.outputs.app-slug }} @@ -129,3 +160,23 @@ jobs: fi git merge --no-ff "origin/${BASE}" -m "Sync ${BASE} back into ${STAGING} after release" git push origin "HEAD:${STAGING}" + + # The alert lives here rather than in each repo's wrapper, which is what + # made every wrapper `if: failure()` and therefore incapable of ever saying + # a failure had been FIXED. Ending the job with this step covers both + # directions in one place for all seven consumers. + # `continue-on-error` because the alert must never decide the job's fate. It + # runs under `always()` on green runs too now, and the Slack action calls + # `core.setFailed` on any Slack-side error, so without this a rotated bot + # token or a bot removed from the channel would report a successful unfreeze + # as a failure. + - name: Notify the engineering channel + if: ${{ always() && job.status != 'cancelled' }} + continue-on-error: true + uses: ./.ci-shared/notify-pipeline-status + with: + env-name: "staging unfreeze" + status: ${{ job.status == 'failure' && 'failed' || 'recovered' }} + github-token: ${{ github.token }} + slack-channel-id: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + slack-bot-token: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} diff --git a/.github/workflows/sync-main-to-staging.yml b/.github/workflows/sync-main-to-staging.yml index 3083f73..8f5fc02 100644 --- a/.github/workflows/sync-main-to-staging.yml +++ b/.github/workflows/sync-main-to-staging.yml @@ -28,6 +28,11 @@ # # Called by a per-repo wrapper, e.g. # auth/.github/workflows/sync-main-to-staging.yml +# The wrapper carries NO notify job: this workflow ends with one. The wrappers +# used to own it, written `if: failure()`, which meant a sync could report that +# it broke and never that it was fixed. On 2026-08-14 cowork's sync failed and +# alerted, the re-run succeeded, and the notify job was SKIPPED, leaving the +# channel holding a red alert for a sync that had already recovered. # # Requires: vars.RELEASE_APP_CLIENT_ID + secrets.RELEASE_APP_PRIVATE_KEY # (org-provisioned; secret reaches here via `secrets: inherit`). @@ -50,8 +55,20 @@ on: type: string default: ubuntu-latest -permissions: - contents: read +# No workflow-level `permissions:` block, deliberately, so this job inherits the +# calling job's grant. That is what makes the notify step's recovery lookup work: +# a block here would be the CEILING for this job's token as well as the caller's, +# so declaring `contents: read` alone would cap the token to `contents: read` and +# the lookup would be refused no matter what the caller granted. Declaring +# `actions: read` instead is not an option either — every caller that has not yet +# granted it would fail to LOAD, and these wrappers reach a repo's default branch +# only at the next weekly release, so the two can never merge in step. Inheriting +# gives both: the grant where a caller has it, and a refused lookup that logs why +# and stays quiet where it does not. +# +# The cost is real and worth naming: `scripts/workflow_graph.py` check 2 compares +# what a callee DECLARES against what its callers grant, so a job that declares +# nothing is invisible to it. That check cannot help here. jobs: sync: @@ -69,6 +86,16 @@ jobs: fetch-depth: 0 token: ${{ steps.app-token.outputs.token }} + # After the checkout above, which would otherwise clean this back out of + # the workspace. + - name: Check out the shared alert implementation + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + with: + repository: mindsdb/github-actions + ref: ${{ job.workflow_sha }} + path: .ci-shared + persist-credentials: false + - name: Sync base branch into staging env: GH_TOKEN: ${{ steps.app-token.outputs.token }} @@ -116,3 +143,20 @@ jobs: echo "::error::Could not fast-forward ${STAGING} after 2 attempts. ${STAGING} is moving faster than this workflow can merge, or the App lost its bypass on ${STAGING}." exit 1 + + # `continue-on-error` because the alert must never decide the job's fate. It + # runs under `always()` on green runs too now, and the Slack action calls + # `core.setFailed` on any Slack-side error, so without this a rotated bot + # token or a bot removed from the channel would report a successful run as a + # failure — and for the freeze that also skips the release-PR refresh, which + # gates on this run's conclusion. + - name: Notify the engineering channel + if: ${{ always() && job.status != 'cancelled' }} + continue-on-error: true + uses: ./.ci-shared/notify-pipeline-status + with: + env-name: "main -> staging sync" + status: ${{ job.status == 'failure' && 'failed' || 'recovered' }} + github-token: ${{ github.token }} + slack-channel-id: ${{ secrets.SLACK_ENG_CHANNEL_ID }} + slack-bot-token: ${{ secrets.GH_ACTIONS_SLACK_BOT_TOKEN }} diff --git a/.gitignore b/.gitignore index 7a60b85..2c15fc1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,6 @@ __pycache__/ *.pyc +# The actionlint binary. `ci.yml` and this repo's own "how to test" steps both +# download it into the repo root, so it turns up untracked in every checkout +# where anyone has run the lint by hand. +/actionlint diff --git a/README.md b/README.md index a09bf6f..1b893a3 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ They live in `.github/workflows/` and are called from ~25-line per-repo wrappers | Reusable workflow | Name (keep identical in callers) | What it does | |---|---|---| | `release-freeze.yml` | `Staging Freeze` | Activates the `staging-freeze` ruleset to lock staging (skips if staging == main) | -| `release-pr.yml` | `Create staging to main release PR` | Opens the `staging → main` PR (idempotent) | +| `release-pr.yml` | `Create staging to main release PR` | Keeps the `staging → main` PR open and current: a draft that lists what is queued, marked ready for review when the freeze opens | | `release-unfreeze.yml` | `Staging Unfreeze` | Disables the ruleset when the release PR merges, then syncs `main` back into `staging` | | `sync-main-to-staging.yml` | `Sync main to staging` | Merges `main` into `staging` after **any** push to main, not just the release merge | @@ -43,6 +43,43 @@ workflow via `workflow_run`; merging that PR fires `Staging Unfreeze`. The `workflow_run` link matches on the **caller** workflow's name, so callers must keep the names above verbatim. +### The release PR is open all week + +The release queue used to be invisible until Friday: the only way to know what the next release would ship was to diff two branches by hand. So the release-PR wrapper also fires after each staging pipeline, and the reusable **updates** an open PR rather than skipping it. The commit list and contributor list are rewritten every run, so the PR is a live view of what would ship right now, from the first merge after an unfreeze. + +**It opens as a draft and becomes ready at freeze time.** An always-open `staging → main` PR is otherwise a merge button sitting next to production all week, and the point of the freeze window is that staging is only a release candidate inside it. Draft is what makes the rolling PR safe, and the draft → ready transition is a visible signal that the window opened. It only ever moves draft → ready: re-drafting a PR a human deliberately marked ready would fight them. + +The second trigger is `workflow_run`, not `push: staging`, deliberately. A second workflow on `push: staging` would be a second disconnected run tree, which the workflow lint forbids. Add the repo's own staging pipeline by name: + +```yaml +on: + workflow_run: + workflows: ["Staging Freeze", "Staging - Build and Deploy on push to staging"] + types: [completed] +``` + +### Wrappers carry no notify job + +All four reusables **post their own Slack alert** as a final step. Wrappers used to own that job, written `if: failure()` with the default `status: failed`, which made all twenty-one of them structurally incapable of reporting that a failure had been FIXED. On 2026-08-14 cowork's `main → staging` sync failed and alerted, the re-run succeeded, and the notify job was *skipped* — leaving the channel holding a red alert for a sync that had already recovered. + +So a wrapper is now triggers plus a `uses:`, and it grants `actions: read` so the recovery lookup works: + +```yaml +jobs: + freeze: + permissions: + contents: read + actions: read # turns the "recovered" message on + uses: mindsdb/github-actions/.github/workflows/release-freeze.yml@main + secrets: inherit +``` + +The four release-train reusables declare **no `permissions:` block at all**, so their job inherits exactly what the calling job grants. That is what makes the grant above work, and getting it wrong is silent: a `permissions:` block in a called workflow is the ceiling for its own token as well, so declaring `contents: read` there caps the token to `contents: read` and the recovery lookup is refused no matter what the caller granted. Declaring `actions: read` instead is not an option either — every caller that has not yet granted it would fail to *load*, and these wrappers reach a repo's default branch only at the next weekly release, so the two can never be merged in step. Inheriting gives both halves: the grant where a caller has it, and a refused lookup that logs why and stays quiet where it does not. + +The cost is worth naming. `scripts/workflow_graph.py` check 2 compares what a callee *declares* against what its callers grant, so a job that declares nothing is invisible to it. That check cannot help on these four. + +`notify-main-failure.yml` is the exception: it *does* declare `actions: read`, at workflow level, and always has. Every caller of it must grant the scope or the run is a `startup_failure`. + ## Merge-to-main panic alerts `notify-main-failure.yml` posts a "pipeline failed" alert to the engineering @@ -88,13 +125,13 @@ missing recovery message is the symptom to look for. A staging failure is not the same event all week. Once the freeze window is open, `staging` is the release candidate and a red pipeline blocks the release. Before it, `staging` is the integration branch and the same red is routine, so paging the channel for it is what teaches people to scroll past the channel. -`freeze-scoped: true` on a **staging** caller implements that policy: +`freeze-scoped: true` on a **staging** caller implements that policy, in *both* directions: -| Where it failed | Freeze window | Result | +| Where it happened | Freeze window | Result | |---|---|---| -| `main` | n/a | red alert | -| `staging` | open | red alert | -| `staging` | not open | nothing posted | +| `main`, failed or recovered | n/a | posted | +| `staging`, failed or recovered | open | posted | +| `staging`, failed or recovered | not open | **nothing posted** | ```yaml with: @@ -103,7 +140,9 @@ A staging failure is not the same event all week. Once the freeze window is open freeze-scoped: true ``` -Add `outside-freeze: notice` to post a muted grey `:warning:` message instead of nothing, for a caller that wants the trail. Silence is the default; the run is still red in the Actions tab either way, and `pipeline-watchdog.yml` still covers a staging branch that stops building at all. +**That last row means both halves.** It used to mean only the failure half, and the recovery posted regardless — so the channel was told a staging pipeline had recovered from a failure it was never told about. On 2026-08-13 cowork-server's staging publish failed on the commit that *fixed* a broken release candidate; the failure was correctly silent, and the re-run three and a half hours later posted a green "Recovered". The only thing anyone heard about a stalled rc stream was the good news. Half a story is worse than none: it reads as noise at best and as a resolved prod incident at worst. + +There is no muted middle setting. Outside the window a staging pipeline posts nothing at all — no red, no green, no grey. The run is still red in the Actions tab, and `pipeline-watchdog.yml` covers both a staging branch that stops building entirely and one that is **still red when the freeze opens**. A workflow triggered on **both** main and staging from one notify job must not hardcode `true`, or main failures get silenced too. Pass an expression: @@ -115,13 +154,13 @@ Freeze state is read from the `staging-freeze` ruleset itself, not inferred from Leave `freeze-scoped` off for prod and freeze/unfreeze callers: a failure there is always worth interrupting for. -For a freeze/unfreeze wrapper, keep it failure-only: `needs:` its single job, -`if: failure()`, the default `status: failed`, no `permissions:` block (the -prior-run lookup only runs in recovered mode), and label it accordingly -(`env-name: "staging freeze"`). Pass `runs-on: ubuntu-latest` for repos without -the self-hosted `mdb-dev` runner. For a workflow that also runs on -`pull_request`, add `&& github.event_name == 'push'` to the `if:` so PR-run -failures (which the author already sees) don't alert the channel. +**Release-train wrappers no longer add a notify job at all** — `release-freeze.yml`, `release-unfreeze.yml`, `sync-main-to-staging.yml` and `release-pr.yml` each end with one. See "Wrappers carry no notify job" above for why, and for the `actions: read` grant they need. + +The alert itself lives in the `notify-pipeline-status` **composite action**, which both shapes of caller run. A pipeline needs a terminal *job* that `needs:` every other job, so it calls this reusable workflow; a release-train workflow owns a single job and simply ends with the composite step. Same copy, same freeze scoping, same recovery detection, one implementation. + +The notify job runs on `ubuntu-latest` by default. It posts a Slack message and needs no cluster, no cloud and no self-hosted anything, and an alert that runs on the self-hosted fleet cannot report that the self-hosted fleet is down — the one outage where every pipeline goes red at once is the one where it would have nothing to run on. + +For a workflow that also runs on `pull_request`, add `&& github.event_name == 'push'` to the `if:` so PR-run failures (which the author already sees) don't alert the channel. Requires two org secrets, reaching the workflow via `secrets: inherit`: `SLACK_ENG_CHANNEL_ID` (the engineering channel; distinct from the deploy-chatter @@ -159,6 +198,26 @@ jobs: secrets: inherit ``` +### The second thing it watches: a branch that is STILL red + +The same watchdog runs a second sweep, `red-branch-sweep` (on by default), for the case where a pipeline failed and *nobody is being told*. Three ways that happens, all of which have: + +| Case | Why the in-run notify job did not cover it | +|---|---| +| Staging broke mid-week | The alert was deliberately silenced, and the branch is still red when Friday's freeze makes it the release candidate | +| It was reported once and stayed broken | One message is easy to miss and there is no second one | +| A single job was re-run | "Re-run failed jobs" re-runs dependents, so notify fires again. The per-job **"Re-run this job"** button does not, so a run can go red to green with notify never running twice | + +It is a **backstop, not an echo**: a finding must be at least `min-age-minutes` old (default 30), which turns the message from "this failed" — which the pipeline already said — into "this is still failing and nobody has touched it". On `staging` it only speaks while the freeze is on, and **the window opening is itself a trigger**, so a branch that broke on Tuesday is reported on Friday. That is keyed on a successful run of the `Staging Freeze` workflow rather than on a clock, so moving the freeze moves the alerting with it and neither has to know about the other. + +`startup_failure` is reported only by the first sweep and excluded from this one, so one incident never produces two alerts. It still counts as the newest run, though, so a startup failure supersedes an older red rather than letting the sweep reach past it and report both. + +**Know how wide it is.** The sweep reads run history, not notify wiring, so by default it reports *any* red workflow on a swept branch — including a scheduled job or a one-off check that never opted into an in-run alert. Pass `workflows:` with the paths that matter to narrow it. The sweep never reports the watchdog itself, which it derives from `github.workflow_ref` rather than asking the caller. + +Age is measured from the current attempt's `run_started_at`, not the run's `created_at`. A re-run keeps the original `created_at` forever, so an age window measured from it filters out the very attempt that just failed — and a re-run is the most common way a failure gets fixed, or re-broken. + +Selection logic lives in `scripts/branch_health.py` with unit tests, because it is date arithmetic plus a two-way trigger and that does not belong in a jq expression. + Alerts repeat, by design: one failing push produces about three messages at that cadence and window, and then silence. Alerting only on the *transition* into a broken state gives exactly one message per break, which was rejected after replaying it against the auth incident — it would have said nothing about the second failing push, since the run before that one had also failed to start. A stateless sweep cannot be exactly-once, so the choice is a message that can be missed or a few that cannot, and the window is what bounds the few. Two things to know when adding it. GitHub only runs `schedule` triggers from a repository's **default branch**, so a watchdog merged to `staging` and no further is inert — `workflow_dispatch` is there to prove it before it reaches `main`. And widening `lookback-minutes` on a manual dispatch replays a past incident, which is how to check it would have caught one. @@ -170,7 +229,7 @@ Two things to know when adding it. GitHub only runs `schedule` triggers from a r | Layer | Blocking | What it is | | --- | --- | --- | | `actionlint` | yes | The established syntax + expression + shellcheck linter | -| permission check | yes | `scripts/workflow_graph.py` — the one gap neither tool covers | +| permission check | yes | `scripts/workflow_graph.py` — the two gaps neither tool covers: one run tree per event, and a callee that declares a permission its caller lacks | | `zizmor` | advisory by default | The established Actions *security* auditor (template injection, credential persistence, unpinned actions) | ```yaml @@ -193,6 +252,28 @@ It also refuses to read cluster secrets by hand — see `k8s-secret` below. Blind spot to know about: it can only read local (`./.github/workflows/...`) callees, and lists remote ones as unchecked. The cap applies to those too, so a scope added to a reusable *here* must be granted by every consumer's calling job. +## The release-freeze contract + +A freeze is one thing: the `enforcement` field of a pre-provisioned repository ruleset, flipped between `active` and `disabled`. Four workflows care — freeze and unfreeze write it, and the two alerting paths read it to decide whether a red staging is worth interrupting anyone for. + +They used to carry three separate copies of that knowledge, and only two of them took the ruleset name as an input; the alerting path hardcoded it. Renaming the ruleset in one repo would therefore have moved the freeze and left the alerting reading a name that no longer existed — silent in the direction that hurts, because a reader that cannot establish the state escalates, so every ordinary mid-week staging red would have paged the channel forever and the cause would have looked like a Slack problem. + +`scripts/freeze_state.py` now owns it, with two modes that fail in opposite directions on purpose: + +| Mode | Used by | On a lookup failure | +|---|---|---| +| `read --on-error escalate` | the alert paths | report frozen, exit 0 — never downgrade a real release-blocking failure, and never redden a green run | +| `read --on-error fail` | `release-pr.yml` | leave the PR a draft — the safe direction there is the opposite one, since "ready" invites a merge of an unvalidated branch | +| `set --enforcement …` | freeze and unfreeze | fail loudly — a freeze that did not apply must stop the release train rather than let the window appear to open | + +The flip is a read-modify-write of the whole ruleset: a partial `PUT` is not guaranteed to preserve the fields it omits, and the omitted fields are the bypass actors and branch conditions, so getting it wrong unlocks the branch it was asked to lock. The body goes to a file and is never echoed, because three of the consuming repos are public and a ruleset body names its bypass actors. + +The four release-train reusables and both notify reusables check these scripts out at `job.workflow_sha` — the commit of the workflow file that defines the running job, so a consumer that pins the workflow to a SHA gets that SHA's scripts too. Pinning a workflow while its scripts float is not a pin. + +Use `job.workflow_sha`, not `github.job_workflow_sha`. The latter looks right, is quoted in GitHub's OIDC documentation, and does not exist in the `github` context — it is a token claim only ([actions/runner#2417](https://github.com/actions/runner/issues/2417)), so it evaluates to empty and `actions/checkout` silently falls back to the default branch. GitHub added `job.workflow_sha`, `job.workflow_ref`, `job.workflow_repository` and `job.workflow_file_path` for exactly this in April 2026. actionlint does not know them yet, hence the scoped ignore in `.github/actionlint.yaml`. + +`workflow-lint.yml` deliberately does **not** pin: it pulls `scripts/workflow_graph.py` from the default branch, so a lint-rule fix reaches every consumer without seven wrapper bumps. + ## Reading a Kubernetes secret `k8s-secret` fetches one key from a Secret, fails when it is absent, masks it, and only then exports it — in that order, which is the part a hand-rolled fetch gets wrong, because `::add-mask::` only scrubs output that comes *after* it. @@ -304,12 +385,16 @@ keeps its own ledger. ### Caller wrappers -Drop these three files into each repo's `.github/workflows/`. Adjust the cron -per repo if desired; branch names default to `staging`/`main`. +Drop these four files into each repo's `.github/workflows/`. Adjust the cron per +repo if desired; branch names default to `staging`/`main`. Each is triggers plus +a `uses:` — **no notify job**, because every reusable posts its own. -> **Pinning:** the examples below use `@main` for readability. For production, -> pin each `uses:` to a full commit SHA with a version comment -> (e.g. `…/release-freeze.yml@ # v1`) so Dependabot can manage bumps. +> **Pinning:** these call `@main`, matching every other call site in this repo. +> The reusables check their own scripts and composite out at `job.workflow_sha`, +> so a caller that *does* pin to a SHA gets that SHA's behaviour end to end +> rather than a pinned workflow running `main`'s logic. Note that the previously pinned wrappers had gone two commits stale and +> were still running a `release-unfreeze.yml` that skipped the sync-back on a +> manual dispatch, months after that was fixed here. `staging-freeze.yml`: @@ -317,9 +402,7 @@ per repo if desired; branch names default to `staging`/`main`. name: Staging Freeze on: schedule: - # Friday 13:47 UTC — off the top of the hour (GitHub's documented high-load - # slot) and off a DST-sensitive "6am PST" wording. - - cron: '47 13 * * 5' + - cron: '47 13 * * 5' # Friday 13:47 UTC, off the top of the hour workflow_dispatch: permissions: @@ -327,52 +410,111 @@ permissions: jobs: freeze: + permissions: + contents: read + actions: read # turns the "recovered" message on uses: mindsdb/github-actions/.github/workflows/release-freeze.yml@main secrets: inherit ``` -`weekly-merge-staging.yml`: +`staging-unfreeze.yml`: ```yaml -name: Create staging to main release PR +name: Staging Unfreeze on: - workflow_run: - workflows: ["Staging Freeze"] - types: [completed] + pull_request: + types: [closed] + branches: [main] workflow_dispatch: permissions: contents: read +concurrency: # shared with sync-main-to-staging.yml + group: sync-main-to-staging + cancel-in-progress: false + jobs: - create-pr: + unfreeze: if: > github.event_name == 'workflow_dispatch' || - github.event.workflow_run.conclusion == 'success' - uses: mindsdb/github-actions/.github/workflows/release-pr.yml@main + (github.event.pull_request.merged == true && + github.event.pull_request.head.ref == 'staging' && + github.event.pull_request.head.repo.full_name == github.repository) + permissions: + contents: read + actions: read + uses: mindsdb/github-actions/.github/workflows/release-unfreeze.yml@main secrets: inherit ``` -`staging-unfreeze.yml`: +`sync-main-to-staging.yml`: ```yaml -name: Staging Unfreeze +name: Sync main to staging + +# run-tree-ok: a release-train hook, not a stage of this repo's push:main +# pipeline. Folding it in would make the sync conditional on that pipeline +# succeeding, which is the exact failure it exists to prevent. + on: - pull_request: - types: [closed] + push: branches: [main] workflow_dispatch: +concurrency: # shared with staging-unfreeze.yml + group: sync-main-to-staging + cancel-in-progress: false + permissions: contents: read jobs: - unfreeze: + sync: + permissions: + contents: read + actions: read + uses: mindsdb/github-actions/.github/workflows/sync-main-to-staging.yml@main + secrets: inherit +``` + +`weekly-merge-staging.yml` — note the second `workflow_run` source, which is +this repo's own staging pipeline **by name**, and is what keeps the release PR +current all week: + +```yaml +name: Create staging to main release PR +on: + workflow_run: + workflows: ["Staging Freeze", "Staging - Build and Deploy on push to staging"] + types: [completed] + workflow_dispatch: + +permissions: + contents: read + +# The find-then-create in the reusable is not atomic, so two overlapping runs +# (the freeze and a staging merge in the same minute) would race. +concurrency: + group: release-pr + cancel-in-progress: false + +jobs: + create-pr: + # Both sources spelled out. A failed FREEZE means staging was never locked, so + # the release PR would be premature. A failed staging PIPELINE says nothing + # about what is queued, so the refresh still runs. The pipeline is checked for + # `head_branch` because it can be dispatched on another ref; the freeze cannot + # be, since a `schedule` run is attributed to the default branch. if: > github.event_name == 'workflow_dispatch' || - (github.event.pull_request.merged == true && - github.event.pull_request.head.ref == 'staging' && - github.event.pull_request.head.repo.full_name == github.repository) - uses: mindsdb/github-actions/.github/workflows/release-unfreeze.yml@main + (github.event.workflow_run.name == 'Staging Freeze' && + github.event.workflow_run.conclusion == 'success') || + (github.event.workflow_run.name == 'Staging - Build and Deploy on push to staging' && + github.event.workflow_run.head_branch == 'staging') + permissions: + contents: read + actions: read + uses: mindsdb/github-actions/.github/workflows/release-pr.yml@main secrets: inherit ``` diff --git a/notify-pipeline-status/action.yml b/notify-pipeline-status/action.yml new file mode 100644 index 0000000..bb123f8 --- /dev/null +++ b/notify-pipeline-status/action.yml @@ -0,0 +1,264 @@ +# One implementation of "tell the engineering channel how a pipeline ended". +# +# It exists as a composite action rather than only as a reusable workflow because +# the two kinds of caller need different shapes and used to get different code: +# +# A pipeline (prod-build-deploy, staging-build-deploy, publish-staging) needs a +# terminal JOB that `needs:` every other job, so it calls +# `notify-main-failure.yml`, which is now a thin wrapper around this. +# +# A release-train workflow (freeze, unfreeze, sync, release-pr) owns its own +# single job and can simply end with this step. Those used to push the alert out +# to a per-repo wrapper job, which is how twenty-one callers ended up written as +# `if: failure()` with no recovery mode at all: they could report that the sync +# broke and never that it was fixed. On 2026-08-14 cowork's main -> staging sync +# failed, alerted, was re-run successfully, and the notify job was SKIPPED, so +# the channel was left holding a red alert for a sync that had already +# succeeded. +# +# Both now run these steps, so a change to how an alert reads or when it is +# suppressed is one edit. +# +# `freeze-scoped` silences a STAGING caller entirely while the release-freeze +# window is closed — no red, no green, no grey. Mid-week staging is the +# integration branch, and posting only the recovery half of a story the channel +# was never told is worse than posting neither. +# +# Secrets and tokens are INPUTS, not `secrets` context: a composite action cannot +# read `secrets`. The App token is minted by the caller rather than here, because +# `continue-on-error` is not available to composite steps and a failure to mint +# must degrade to "assume frozen" rather than fail the job. +# +# WHETHER it posts and HOW it reads live in `scripts/notify_decision.py`, not in +# this file's `if:` expressions. Five inputs and three outcomes is more than a +# YAML condition can carry readably, and none of it was reachable by a test while +# it lived here — which is how the `workflow_dispatch` defect below survived +# review. `force-post` is now an explicit input rather than something inferred +# from `github.event_name`: a called workflow inherits the CALLER's context, so +# an event check here cannot tell "the notify workflow was dispatched as a smoke +# test" from "somebody manually ran a release-train wrapper", and all four of +# those wrappers declare `workflow_dispatch`. + +name: Notify pipeline status +description: Post a pipeline failure or recovery to the engineering Slack channel, scoped to the release-freeze window. + +inputs: + env-name: + description: "Short label for the pipeline (e.g. 'prod build+deploy', 'staging freeze')" + required: true + status: + description: "'failed' (red alert) or 'recovered' (green, only if the prior run was failing)" + required: false + default: failed + freeze-scoped: + description: "'true' to stay silent entirely while the staging freeze window is closed" + required: false + default: "false" + ruleset-name: + description: "The freeze ruleset to read" + required: false + default: staging-freeze + freeze-token: + description: "A release-train App token that can read rulesets. Empty means 'could not mint', which escalates to frozen." + required: false + default: "" + github-token: + description: "Token for the prior-run lookup behind the recovery message. Needs actions:read." + required: false + default: "" + force-post: + description: "'true' to post a recovery with no prior failure to recover from. The smoke test, and nothing else: a release-train wrapper must leave this alone or every manual run reports a recovery that did not happen." + required: false + default: "false" + branch: + description: "Branch to name in the message. Defaults to the running ref, which is wrong for a `workflow_run` caller, where GITHUB_REF is the default branch rather than the branch whose pipeline finished." + required: false + default: ${{ github.ref_name }} + shared-path: + description: "Where mindsdb/github-actions is checked out, for scripts/freeze_state.py" + required: false + default: .ci-shared + slack-channel-id: + description: "The engineering channel" + required: true + slack-bot-token: + description: "The Slack bot token" + required: true + +runs: + using: composite + steps: + # The freeze state is read FIRST and vetoes everything below it, including + # the prior-run lookup that recovery mode would otherwise spend an API call + # on. + # + # The signal is the repository ruleset that the freeze/unfreeze workflows + # toggle, not an inference from workflow-run history: a freeze that skipped + # itself because staging had nothing unreleased still concludes `success`, + # and history cannot tell that apart from a real freeze. + # + # `scripts/freeze_state.py` is also what those workflows use to WRITE it, so + # one file owns the ruleset contract and renaming a ruleset is one edit + # rather than four that agree by luck. + - name: Check release-freeze state + id: freeze + if: inputs.freeze-scoped == 'true' + shell: bash + env: + GH_TOKEN: ${{ inputs.freeze-token }} + REPO: ${{ github.repository }} + RULESET_NAME: ${{ inputs.ruleset-name }} + SHARED: ${{ inputs.shared-path }} + run: | + set -uo pipefail + # Every failure path lands on frozen=true. A notify step that cannot + # establish the freeze state must escalate rather than silently downgrade + # a real release-blocking failure; being wrong in this direction costs one + # unnecessary alert. + if [ -z "${GH_TOKEN:-}" ]; then + echo "::warning::No release-train App token. Treating the branch as frozen so failures still escalate." + echo "frozen=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + python3 "${SHARED}/scripts/freeze_state.py" read \ + --repo "${REPO}" --ruleset-name "${RULESET_NAME}" --on-error escalate + + # For the recovery message, find out how the PREVIOUS conclusive run of this + # same workflow on this same branch ended. This step only gathers evidence; + # `notify_decision.py` below decides what it means. + # + # Skipped when nothing it could find would change the outcome: a failure posts + # regardless, `force-post` posts regardless, and a freeze-scoped caller outside + # the window posts nothing either way. `steps.freeze.outputs.frozen` is empty + # when the freeze step did not run, which is not 'false', so an unscoped caller + # still reaches this. + # + # BRANCH is deliberately the running ref rather than `inputs.branch`. For a + # `workflow_run` caller those differ: the run is attributed to the default + # branch, so that is where its own history lives, while the message wants the + # branch whose pipeline finished. + - name: Check how the previous run ended + id: prev + if: inputs.status == 'recovered' && inputs.force-post != 'true' && steps.freeze.outputs.frozen != 'false' + shell: bash + env: + GH_TOKEN: ${{ inputs.github-token }} + REPO: ${{ github.repository }} + WORKFLOW: ${{ github.workflow }} + WORKFLOW_REF: ${{ github.workflow_ref }} + BRANCH: ${{ github.ref_name }} + RUN_ID: ${{ github.run_id }} + RUN_ATTEMPT: ${{ github.run_attempt }} + run: | + set -uo pipefail + + # A RE-RUN is the most common way a failure gets fixed, and it was the one + # case this could never report. Attempts share a run id, so the failing + # attempt is the very run the history lookup below excludes as "the + # current one" — it would then find some older, green run and conclude + # nothing had broken. Check the previous attempt first. + if [ "${RUN_ATTEMPT:-1}" -gt 1 ]; then + PREV_ATTEMPT=$((RUN_ATTEMPT - 1)) + if LAST=$(gh api "repos/${REPO}/actions/runs/${RUN_ID}/attempts/${PREV_ATTEMPT}" \ + --jq '.conclusion // ""' 2>&1); then + echo "prev_conclusion=${LAST}" >> "$GITHUB_OUTPUT" + echo "Attempt ${PREV_ATTEMPT} concluded '${LAST:-unknown}'." + exit 0 + fi + echo "Could not read attempt ${PREV_ATTEMPT}, falling back to run history: ${LAST}" + fi + + # Match the workflow FILE rather than its display name, which a + # `run-name:` override changes; fall back to the name if the ref does not + # resolve to a path inside this repo. + WF_PATH="${WORKFLOW_REF%%@*}" + export WF_PATH="${WF_PATH#"${REPO}/"}" + + # Most recent CONCLUSIVE run on this branch, excluding the current one: + # cancelled and skipped runs are not evidence either way. + if ! PREV=$(gh api "repos/${REPO}/actions/runs?branch=${BRANCH}&status=completed&per_page=50" \ + --jq '[ .workflow_runs[] + | select(.path == env.WF_PATH or .name == env.WORKFLOW) + | select((.id|tostring) != env.RUN_ID) + | select(.conclusion == "success" or .conclusion == "failure" + or .conclusion == "timed_out" or .conclusion == "startup_failure") + ][0].conclusion // ""' 2>&1); then + # Almost always a missing `actions: read` grant on the caller job. Report + # no evidence rather than failing: an unreadable history has to read as + # "not a recovery", because a notify step must never redden a green run. + echo "Could not read run history, so there is no evidence either way: ${PREV}" + exit 0 + fi + + echo "prev_conclusion=${PREV}" >> "$GITHUB_OUTPUT" + echo "Previous conclusive run on ${BRANCH} concluded '${PREV:-none}'." + + # One decision, one place, with tests. See scripts/notify_decision.py for the + # three outcomes and why `force-post` is an input rather than an event check. + - name: Decide whether to post, and how it reads + id: msg + shell: bash + env: + SHARED: ${{ inputs.shared-path }} + STATUS: ${{ inputs.status }} + FREEZE_SCOPED: ${{ inputs.freeze-scoped }} + FROZEN: ${{ steps.freeze.outputs.frozen }} + FORCE_POST: ${{ inputs.force-post }} + PREV_CONCLUSION: ${{ steps.prev.outputs.prev_conclusion }} + run: | + set -euo pipefail + python3 "${SHARED}/scripts/notify_decision.py" \ + --status "${STATUS}" \ + --freeze-scoped "${FREEZE_SCOPED}" \ + --frozen "${FROZEN}" \ + --force-post "${FORCE_POST}" \ + --prev-conclusion "${PREV_CONCLUSION}" + + - name: Notify Slack + # The payload carries a top-level `text` as well as the blocks. Slack renders + # the blocks, but `text` is what a push notification, a screen reader, and a + # notification-list preview show, so without it the alert arrives on a phone + # as an empty message — the one context where a pipeline failure most needs + # to be readable. + if: steps.msg.outputs.post == 'true' + uses: slackapi/slack-github-action@v1.26.0 + with: + channel-id: ${{ inputs.slack-channel-id }} + payload: | + { + "text": "${{ steps.msg.outputs.prefix }}: ${{ github.event.repository.name }} ${{ inputs.env-name }} pipeline on ${{ inputs.branch }}", + "attachments": [ + { + "color": "${{ steps.msg.outputs.color }}", + "blocks": [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": "${{ steps.msg.outputs.icon }} *<${{ github.server_url }}/${{ github.repository }}|${{ github.event.repository.name }}>* — *${{ inputs.env-name }}* pipeline ${{ steps.msg.outputs.verb }} on `${{ inputs.branch }}`" + }, + "fields": [ + { + "type": "mrkdwn", + "text": "*Workflow*\n<${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}|${{ github.workflow }}>" + }, + { + "type": "mrkdwn", + "text": "*${{ inputs.status == 'recovered' && 'Fixed by' || 'Triggered by' }}*\n${{ github.triggering_actor }}" + }, + { + "type": "mrkdwn", + "text": "*Commit*\n<${{ github.server_url }}/${{ github.repository }}/commit/${{ github.sha }}|${{ github.sha }}>" + }, + { + "type": "mrkdwn", + "text": "*Branch*\n<${{ github.server_url }}/${{ github.repository }}/tree/${{ inputs.branch }}|${{ inputs.branch }}>" + } + ] + } + ] + } + ] + } + env: + SLACK_BOT_TOKEN: ${{ inputs.slack-bot-token }} diff --git a/scripts/branch_health.py b/scripts/branch_health.py new file mode 100644 index 0000000..651c6b4 --- /dev/null +++ b/scripts/branch_health.py @@ -0,0 +1,332 @@ +"""Find deploy branches whose newest pipeline run is red, from outside the run. + +The pipeline's own terminal notify job reports a failure once, at the moment it +happens, and that is almost always enough. Three cases it does not cover, all of +which have now happened: + +1. Nobody was going to be told in the first place. A staging failure outside the + release-freeze window is deliberately silent, because mid-week staging is the + integration branch and paging the channel for it is what teaches people to + scroll past the channel. But the branch is still red when Friday's freeze turns + it into the release candidate, and at that moment nothing says so. + +2. The alert was sent and then the branch stayed red. One message at the moment of + breakage is easy to miss, and there is no second one. + +3. The re-run did not re-notify. GitHub's "Re-run failed jobs" re-runs the failed + job and everything downstream of it, so the terminal notify job fires again and + reports the recovery. The per-job "Re-run this job" button does not: it re-runs + that job alone, so a run can go from red to green with the notify job never + running a second time. There is no hook to fix that from inside the run. + +All three are the same shape: the truth about the branch is in the run history, +and nothing is reading it. So this reads it, on the schedule the pipeline watchdog +already runs on. + +**It is a backstop, not an echo.** A red pipeline has already alerted from inside +its own run, so repeating that within seconds would train people to ignore both. +A finding has to be at least ``--min-age-minutes`` old, which turns the message +from "this failed" into "this is STILL failing and nobody has touched it". The one +exception is the freeze window opening, where the finding is old by definition and +the news is the window, not the failure. + +What it deliberately does NOT report is ``startup_failure``. That is the case +where GitHub rejected the run at load time and no job ran at all, which +``notify-startup-failure`` already sweeps for with a different message and a +different thing to check. Splitting them keeps one finding from producing two +alerts. + +When it fires, per branch: + +- **main** — the newest conclusive run of a workflow is red, it started inside the + lookback window, and it is at least ``--min-age-minutes`` old. Bounded + repetition, the same reading the startup sweep uses: a couple of messages per + breakage at a 30-minute cadence and a 90-minute window, then silence, because a + stateless sweep cannot be exactly-once and a missed alert is the failure being + fixed. + +- **staging** — the same, and additionally only while the release freeze is on, + because that is the window in which a red staging blocks a release. The window + opening is itself a trigger: a branch that went red on Tuesday and is still red + when Friday's freeze lands is reported then, which is the entire point, and the + age floor does not apply to it. + +The freeze workflow's own runs are read separately rather than from a branch +listing. A ``schedule`` trigger runs from the repository's DEFAULT branch, so the +freeze runs are attributed to ``main`` and are not in the ``staging`` history at +all. Looking for them there finds nothing, forever, silently. +""" + +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from datetime import datetime, timedelta, timezone +from typing import Callable, Iterable, Sequence + +Runner = Callable[[Sequence[str]], "subprocess.CompletedProcess[str]"] + +# Conclusions that say something about the branch. `cancelled` and `skipped` are +# evidence of nothing, so they may not hide a failure behind them. +# +# `startup_failure` is CONCLUSIVE but not RED, and the asymmetry is deliberate. +# It belongs to the other sweep, so this one never reports it — but it is still the +# newest thing that happened, so it has to be able to supersede an older failure. +# Leaving it out of CONCLUSIVE entirely made the sweep reach past it and report a +# failure that a later run had already replaced, giving two alerts for one branch. +CONCLUSIVE = ("success", "failure", "timed_out", "startup_failure") +RED = ("failure", "timed_out") + +# The freeze workflow's per-repo wrapper keeps this name verbatim, because the +# release-PR workflow chains off it by name. That makes it a stable handle for +# "the window just opened" without this file knowing anything about the schedule, +# which is the point: move the freeze and the alerting follows. +FREEZE_WORKFLOW_NAME = "Staging Freeze" + + +def _run(argv: Sequence[str]) -> "subprocess.CompletedProcess[str]": + return subprocess.run(list(argv), capture_output=True, text=True, check=False) + + +def parse_time(value: str) -> datetime: + return datetime.fromisoformat(value.replace("Z", "+00:00")) + + +def started_at(run: dict) -> datetime: + """When the run's CURRENT attempt began. + + Not ``created_at``, which stays pinned to attempt 1 forever. A re-run hours + later keeps the original ``created_at``, so an age window measured from it + filters out the attempt that just failed — measured against live runs, a + cowork-server attempt 2 had ``created_at`` 22:26:37 and ``run_started_at`` + 23:47:11, already 80 minutes outside a 90-minute lookback before it started. + Since a re-run is the most common way a failure gets fixed (and re-broken), + that was the case this sweep most needed to see. + + ``created_at`` is the fallback because the freeze-run listing this module also + parses is trimmed to the fields it needs. + """ + return parse_time(run.get("run_started_at") or run["created_at"]) + + +def newest_conclusive_per_workflow(runs: Iterable[dict]) -> list[dict]: + """One run per workflow file: the most recent that concluded either way. + + Keyed on ``path`` rather than ``name`` because a ``run-name:`` override + changes the display name and the path is stable. + """ + newest: dict[str, dict] = {} + for run in runs: + if run.get("conclusion") not in CONCLUSIVE: + continue + path = run.get("path") or run.get("name") or "" + current = newest.get(path) + if current is None or started_at(run) > started_at(current): + newest[path] = run + return sorted(newest.values(), key=lambda run: run["path"]) + + +def freeze_opened_within(freeze_runs: Iterable[dict], *, cutoff: datetime) -> bool: + """True when the freeze workflow last succeeded inside the lookback window. + + This is what turns "staging has been red since Tuesday" into an alert on + Friday. It reads the freeze workflow's own run history, so the trigger moves + whenever the freeze moves. + """ + for run in freeze_runs: + if run.get("conclusion") != "success": + continue + if parse_time(run["created_at"]) >= cutoff: + return True + return False + + +def in_scope(path: str, *, only: Sequence[str], exclude: Sequence[str]) -> bool: + """Whether this workflow file is one the sweep speaks about. + + ``only`` empty means every workflow on the branch, which is the default and is + the widest this gets. It is worth knowing how wide that is: the sweep reads run + history, not the notify wiring, so it reports any red workflow on a deploy + branch and not only the pipelines that opted into an in-run alert. A repo that + wants it narrowed passes `workflows:` with the paths that matter. + + ``exclude`` always carries the sweep's own workflow. A watchdog whose own run + went red would otherwise report itself on the next tick, which reads as a + pipeline failure and is really just the watchdog. + """ + if path in exclude: + return False + return not only or path in only + + +def select_red( + runs: list[dict], + *, + branch: str, + cutoff: datetime, + settled: datetime, + frozen: bool, + staging_branch: str, + freeze_runs: Iterable[dict] = (), + only: Sequence[str] = (), + exclude: Sequence[str] = (), +) -> list[dict]: + """The red pipelines on this branch that are worth a message right now. + + ``cutoff`` bounds how far back a finding may be (so the sweep stops eventually). + ``settled`` bounds how RECENT it may be (so the sweep is not an echo of the + alert the run already sent for itself). + """ + if branch == staging_branch and not frozen: + return [] + + just_froze = branch == staging_branch and freeze_opened_within(freeze_runs, cutoff=cutoff) + + findings = [] + for run in newest_conclusive_per_workflow(runs): + if run["conclusion"] not in RED: + continue + if not in_scope(run["path"], only=only, exclude=exclude): + continue + began = started_at(run) + if not just_froze and not (cutoff <= began <= settled): + continue + findings.append( + { + "id": run["id"], + "path": run["path"], + "name": run["name"], + "conclusion": run["conclusion"], + "branch": branch, + "head_sha": run["head_sha"], + "html_url": run["html_url"], + "actor": (run.get("triggering_actor") or {}).get("login", "unknown"), + "title": run.get("display_title", ""), + # Says which of the two reasons produced this finding, so the + # message can explain itself rather than looking like a repeat. + "reason": "freeze-opened" if just_froze else "still-red", + } + ) + return findings + + +def api(runner: Runner, path: str): + result = runner(["gh", "api", path]) + if result.returncode != 0: + raise RuntimeError(result.stderr.strip() or result.stdout.strip()) + return json.loads(result.stdout) + + +def fetch_runs(repo: str, branch: str, *, runner: Runner = _run) -> list[dict]: + payload = api(runner, f"repos/{repo}/actions/runs?branch={branch}&status=completed&per_page=100") + return payload.get("workflow_runs", []) + + +def fetch_freeze_runs(repo: str, *, runner: Runner = _run) -> list[dict]: + """Recent runs of the freeze workflow, found by its name rather than a path. + + Queried through the workflow's own runs endpoint because a scheduled run is + attributed to the default branch, so these never appear in a `staging` + listing. Returns an empty list when the repo has no freeze workflow, which is + the normal state for a repo that is not on the release train. + """ + workflows = api(runner, f"repos/{repo}/actions/workflows?per_page=100").get("workflows", []) + for workflow in workflows: + if workflow.get("name") == FREEZE_WORKFLOW_NAME: + payload = api( + runner, f"repos/{repo}/actions/workflows/{workflow['id']}/runs?status=completed&per_page=10" + ) + return payload.get("workflow_runs", []) + return [] + + +def main(argv: list[str] | None = None, *, runner: Runner = _run, now: datetime | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo", required=True) + parser.add_argument("--branches", required=True, help="space separated") + parser.add_argument("--staging-branch", default="staging") + parser.add_argument("--lookback-minutes", type=int, default=90) + parser.add_argument( + "--min-age-minutes", + type=int, + default=30, + help="how long a failure must have stood before this repeats it, so the sweep is a backstop rather than an echo", + ) + parser.add_argument( + "--frozen", default="false", help="freeze state of the staging branch, from freeze_state.py" + ) + parser.add_argument( + "--workflows", + default="", + help="space-separated workflow paths to report on; empty means every workflow on the branch", + ) + parser.add_argument( + "--self-path", + default="", + help="this sweep's own workflow path, never reported (a watchdog that went red reports itself otherwise)", + ) + parser.add_argument("--out", default="red-branches.json") + args = parser.parse_args(argv) + + moment = now or datetime.now(timezone.utc) + cutoff = moment - timedelta(minutes=args.lookback_minutes) + settled = moment - timedelta(minutes=args.min_age_minutes) + frozen = args.frozen == "true" + + branches = args.branches.split() + only = args.workflows.split() + exclude = [args.self_path] if args.self_path else [] + freeze_runs: list[dict] = [] + if frozen and args.staging_branch in branches: + try: + freeze_runs = fetch_freeze_runs(args.repo, runner=runner) + except (RuntimeError, json.JSONDecodeError, KeyError) as exc: + print(f"::warning::Could not read freeze workflow history: {exc}") + + findings: list[dict] = [] + unreadable: list[str] = [] + for branch in branches: + try: + runs = fetch_runs(args.repo, branch, runner=runner) + except (RuntimeError, json.JSONDecodeError) as exc: + # Loud in the summary, quiet in the exit code. A watchdog that + # reddens the repo when the API is briefly unhappy gets muted, and a + # muted watchdog is worse than no watchdog. + # + # Recorded rather than only logged, because the step summary used to + # print "No deploy branch is sitting red" on this path — an affirmative + # all-clear for a branch the sweep knows nothing about. + print(f"::warning::Could not read run history for {branch}: {exc}") + unreadable.append(branch) + continue + findings.extend( + select_red( + runs, + branch=branch, + cutoff=cutoff, + settled=settled, + frozen=frozen, + staging_branch=args.staging_branch, + freeze_runs=freeze_runs, + only=only, + exclude=exclude, + ) + ) + + with open(args.out, "w", encoding="utf-8") as handle: + json.dump(findings, handle) + + output = os.environ.get("GITHUB_OUTPUT") + if output: + with open(output, "a", encoding="utf-8") as handle: + handle.write(f"count={len(findings)}\n") + handle.write(f"unreadable={' '.join(unreadable)}\n") + print(f"count={len(findings)} unreadable={' '.join(unreadable) or 'none'}", file=sys.stderr) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/freeze_state.py b/scripts/freeze_state.py new file mode 100644 index 0000000..99f4500 --- /dev/null +++ b/scripts/freeze_state.py @@ -0,0 +1,210 @@ +"""The one place that knows how a repository's release freeze is stored. + +A release freeze is a pre-provisioned repository ruleset (by default named +``staging-freeze``) whose ``enforcement`` field is flipped between ``active`` and +``disabled``. Four workflows care about that fact and they used to each carry +their own copy of it: the freeze and unfreeze workflows flipped it, and the two +alerting workflows read it to decide whether a red staging branch is worth +interrupting anyone for. + +Three copies agreed by luck rather than by contract. Only two of them took the +ruleset name as an input, so renaming the ruleset in one repository moved the +freeze and left the alerting reading a name that no longer existed. That failure +is silent in the direction that hurts: the reader escalates when it cannot +establish the state, so every ordinary mid-week staging red would have paged the +channel forever and the cause would have looked like a Slack problem. + +So the contract lives here, once, and the workflows call it. + +Two modes, because the two callers want opposite things from a failure. + +``read`` answers "is this repository frozen right now" for an alerting workflow. +Its ``--on-error escalate`` default reports frozen and exits 0, because an alert +path that cannot establish the state must escalate rather than silently downgrade +a real release-blocking failure, and because a notify job must never turn a green +pipeline red. + +``set`` flips enforcement for the freeze and unfreeze workflows, and fails loudly. +A freeze that could not be applied has to stop the release train rather than let +the window appear to open. + +The flip is read-modify-write against the whole ruleset. A partial ``PUT`` is not +guaranteed to preserve the fields it omits, and the fields being omitted here are +the bypass actors and the branch conditions, so getting that wrong unlocks the +branch it was asked to lock. The body is written to a file and never echoed: +three of the consuming repositories are public, and a ruleset body names its +bypass actors. +""" + +from __future__ import annotations + +import argparse +import json +import os +import subprocess +import sys +from typing import Callable, Sequence + +# A `gh api` invocation, injectable so the tests do not need a network or a token. +Runner = Callable[[Sequence[str]], "subprocess.CompletedProcess[str]"] + +ENFORCEMENT_ACTIVE = "active" +ENFORCEMENT_DISABLED = "disabled" + + +class LookupError_(Exception): + """The ruleset could not be read. Carries the message the caller should print.""" + + +def _run(argv: Sequence[str]) -> "subprocess.CompletedProcess[str]": + return subprocess.run(list(argv), capture_output=True, text=True, check=False) + + +def fetch_ruleset(repo: str, name: str, *, runner: Runner = _run) -> dict: + """The ruleset named ``name`` in ``repo``, as a dict. + + Raises ``LookupError_`` when the API call fails or no ruleset carries that + name. The two are distinct messages on purpose: "the token cannot read + rulesets" and "provisioning has drifted" get fixed by different people. + """ + listing = runner(["gh", "api", f"repos/{repo}/rulesets"]) + if listing.returncode != 0: + raise LookupError_(f"Could not read rulesets: {listing.stderr.strip() or listing.stdout.strip()}") + + try: + rulesets = json.loads(listing.stdout or "[]") + except json.JSONDecodeError as exc: + raise LookupError_(f"Ruleset listing was not JSON: {exc}") from exc + + for ruleset in rulesets: + if ruleset.get("name") == name: + return ruleset + raise LookupError_(f"Ruleset '{name}' not found in {repo}") + + +def is_frozen(repo: str, name: str, *, runner: Runner = _run) -> bool: + """True when the freeze ruleset is enforced, i.e. the branch is locked.""" + return fetch_ruleset(repo, name, runner=runner).get("enforcement") == ENFORCEMENT_ACTIVE + + +def set_enforcement( + repo: str, + name: str, + enforcement: str, + *, + body_path: str, + runner: Runner = _run, +) -> int: + """Flip the ruleset's enforcement, preserving every other field. + + Returns the ruleset id so the caller can name it in its log line. + """ + ruleset = fetch_ruleset(repo, name, runner=runner) + ruleset_id = ruleset["id"] + + # Re-read the full ruleset rather than reusing the listing entry: the list + # endpoint returns a summary that omits `rules` and `bypass_actors`, and + # PUTting that summary back would drop them. + detail = runner(["gh", "api", f"repos/{repo}/rulesets/{ruleset_id}"]) + if detail.returncode != 0: + # Both streams, because `gh` reports an API error body on stdout while + # writing its own diagnostics to stderr, and a release-train-stopping + # error with no cause in it is the worst kind to be paged about. + raise LookupError_( + f"Could not read ruleset {ruleset_id}: " + f"{detail.stderr.strip() or detail.stdout.strip()}" + ) + + try: + full = json.loads(detail.stdout) + except json.JSONDecodeError as exc: + # Guarded for the same reason the listing above is: an unparseable body + # has to arrive as the "provisioning has drifted" annotation `main()` + # prints, not as a traceback the operator has to read past. + raise LookupError_(f"Ruleset {ruleset_id} was not JSON: {exc}") from exc + payload = { + "name": full["name"], + "target": full["target"], + "enforcement": enforcement, + "bypass_actors": full.get("bypass_actors", []), + "conditions": full.get("conditions", {}), + "rules": full.get("rules", []), + } + with open(body_path, "w", encoding="utf-8") as handle: + json.dump(payload, handle) + + put = runner( + ["gh", "api", "--method", "PUT", f"repos/{repo}/rulesets/{ruleset_id}", "--input", body_path] + ) + if put.returncode != 0: + raise LookupError_( + f"Could not update ruleset {ruleset_id}: " + f"{put.stderr.strip() or put.stdout.strip()}" + ) + return ruleset_id + + +def emit(key: str, value: str) -> None: + """Write a step output, and echo it so the run log shows the decision.""" + output = os.environ.get("GITHUB_OUTPUT") + if output: + with open(output, "a", encoding="utf-8") as handle: + handle.write(f"{key}={value}\n") + print(f"{key}={value}") + + +def main(argv: list[str] | None = None, *, runner: Runner = _run) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("mode", choices=["read", "set"]) + parser.add_argument("--repo", required=True, help="owner/name") + parser.add_argument("--ruleset-name", default="staging-freeze") + parser.add_argument( + "--enforcement", + choices=[ENFORCEMENT_ACTIVE, ENFORCEMENT_DISABLED], + help="set mode only: the enforcement to write", + ) + parser.add_argument( + "--on-error", + choices=["escalate", "fail"], + default="escalate", + help="read mode only: what an unreadable ruleset means", + ) + parser.add_argument("--body-path", default=None, help="set mode only: where to stage the PUT body") + args = parser.parse_args(argv) + + if args.mode == "read": + try: + frozen = is_frozen(args.repo, args.ruleset_name, runner=runner) + except LookupError_ as exc: + if args.on_error == "fail": + # No ::error:: annotation: every caller of this mode catches the + # non-zero exit and degrades deliberately (release-pr.yml leaves + # the PR a draft), so annotating would mark a run red that the + # workflow treats as handled. The caller emits its own ::warning::. + print(f"Could not establish the freeze state: {exc}", file=sys.stderr) + return 1 + # Escalating is the safe direction: treating an unknown state as + # frozen costs one unnecessary alert, treating it as thawed costs + # the release-blocking alert this exists to send. + print(f"::warning::{exc}. Treating the branch as frozen so failures still escalate.") + emit("frozen", "true") + return 0 + emit("frozen", "true" if frozen else "false") + return 0 + + if not args.enforcement: + parser.error("set mode requires --enforcement") + body_path = args.body_path or os.path.join(os.environ.get("RUNNER_TEMP", "."), "ruleset.json") + try: + ruleset_id = set_enforcement( + args.repo, args.ruleset_name, args.enforcement, body_path=body_path, runner=runner + ) + except LookupError_ as exc: + print(f"::error::{exc} — provisioning has drifted.", file=sys.stderr) + return 1 + print(f"Ruleset '{args.ruleset_name}' (#{ruleset_id}) enforcement set to {args.enforcement}.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/notify_decision.py b/scripts/notify_decision.py new file mode 100644 index 0000000..55619ab --- /dev/null +++ b/scripts/notify_decision.py @@ -0,0 +1,143 @@ +"""Whether an alert posts, and how it reads. + +This is the decision the `notify-pipeline-status` composite used to make in two +`if:` expressions and a shell `if/elif/else`. It moved here for the same reason +`freeze_state.py` and `branch_health.py` did: it is the whole behaviour of the +alerting system, it has five inputs and three outcomes, and none of it was +reachable by a test while it lived in YAML. + +Splitting it also fixed a real defect. The composite decided "always post, skip +the prior-run check" from ``github.event_name == 'workflow_dispatch'``, which was +written for the smoke-test dispatch of ``notify-main-failure.yml`` itself. A +called workflow inherits the CALLER's context, so that condition was also true +whenever anyone manually ran one of the four release-train wrappers — and every +one of them declares ``workflow_dispatch``. A successful manual "Staging Freeze" +therefore posted a green ``Recovered`` for a failure that had never happened. +The smoke test now says so explicitly with ``--force-post`` instead of being +inferred from an event name that cannot tell the two cases apart. + +The three outcomes: + +``silenced`` + A freeze-scoped caller while the window is closed. No red, no green, no grey. + Mid-week staging is the integration branch, and posting only the recovery + half of a story the channel was never told is worse than posting neither. + +``alert`` + A failure. Always posts, because the freeze veto above is the only thing that + may suppress one. + +``recovered`` + A success whose predecessor failed. Posts only on that evidence, or when + ``--force-post`` says this is the smoke test. A green run after a green run + posts nothing, which is what keeps the channel worth reading. + +``--prev-conclusion`` is the conclusion of the previous conclusive run, or of the +previous attempt of this run. Empty means "no evidence": either the lookup was +never made because nothing was going to post anyway, or it was refused. Refused +is the common one — the recovery lookup needs ``actions: read`` on the calling +job — and it has to read as "not a recovery" rather than as an error, because a +notify step must never turn a green pipeline red. +""" + +from __future__ import annotations + +import argparse +import os +import sys + +# Conclusions a `success` can be a recovery FROM. `cancelled` and `skipped` are +# not evidence that anything was broken, so recovering from them is not news. +FAILED_CONCLUSIONS = frozenset({"failure", "timed_out", "startup_failure"}) + +STYLES: dict[str, dict[str, str]] = { + "silenced": {"color": "", "icon": "", "verb": "", "prefix": ""}, + "recovered": {"color": "#00C851", "icon": ":white_check_mark:", "verb": "recovered", "prefix": "Recovered"}, + "alert": {"color": "#FF4444", "icon": ":rotating_light:", "verb": "failed", "prefix": "FAILED"}, +} + + +def decide( + *, + status: str, + freeze_scoped: str, + frozen: str, + force_post: str, + prev_conclusion: str, +) -> dict[str, str]: + """The full decision, as the flat strings the Slack payload interpolates. + + Every argument arrives as a string because that is what a composite action's + inputs and a step's outputs are. ``frozen`` is ``""`` when the freeze step did + not run, which is not the same as ``"false"``: only an actually-thawed + freeze-scoped caller is silenced. + """ + silenced = freeze_scoped == "true" and frozen == "false" + + if silenced: + level, post = "silenced", False + elif status == "recovered": + level = "recovered" + post = force_post == "true" or prev_conclusion in FAILED_CONCLUSIONS + else: + level, post = "alert", True + + return {"post": "true" if post else "false", "level": level, **STYLES[level]} + + +def why(decision: dict[str, str], *, status: str, force_post: str, prev_conclusion: str) -> str: + """One line for the run log, because a silent no-op is hard to debug.""" + if decision["level"] == "silenced": + return "Freeze-scoped and the window is closed, so nothing posts in either direction." + if decision["level"] == "alert": + return "Posting a failure." + if force_post == "true": + return "Posting a recovery unconditionally (--force-post: this is the smoke test)." + if decision["post"] == "true": + return f"Previous run concluded '{prev_conclusion}', so this is a recovery. Posting." + return ( + f"Previous run concluded '{prev_conclusion or 'unknown'}', which is not a failure, " + "so this is an ordinary green run. Staying quiet." + ) + + +def emit(decision: dict[str, str]) -> None: + output = os.environ.get("GITHUB_OUTPUT") + if output: + with open(output, "a", encoding="utf-8") as handle: + for key, value in decision.items(): + handle.write(f"{key}={value}\n") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--status", default="failed", help="'failed' or 'recovered'") + parser.add_argument("--freeze-scoped", default="false") + parser.add_argument( + "--frozen", + default="", + help="'true'/'false' from freeze_state.py, or empty when the freeze was not read", + ) + parser.add_argument( + "--force-post", + default="false", + help="post a recovery without evidence; set only by the notify workflow's own smoke-test dispatch", + ) + parser.add_argument("--prev-conclusion", default="", help="empty means no evidence either way") + args = parser.parse_args(argv) + + decision = decide( + status=args.status, + freeze_scoped=args.freeze_scoped, + frozen=args.frozen, + force_post=args.force_post, + prev_conclusion=args.prev_conclusion, + ) + emit(decision) + print(why(decision, status=args.status, force_post=args.force_post, prev_conclusion=args.prev_conclusion)) + print(f"level={decision['level']} post={decision['post']}", file=sys.stderr) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/workflow_graph.py b/scripts/workflow_graph.py index 9371707..4d22da8 100644 --- a/scripts/workflow_graph.py +++ b/scripts/workflow_graph.py @@ -372,30 +372,69 @@ def _walk( return found +# What `pull_request` fires on when a workflow does not say. Needed because a +# workflow that declares nothing and one that declares `types: [closed]` are +# DISJOINT, and treating "unspecified" as "everything" reported them as a clash. +# That false positive is not academic: it is most of what this check found in the +# repos that had a PR-close cleanup workflow, and an exemption comment was the +# only way to silence it, which is how a real finding later gets waved through. +PULL_REQUEST_DEFAULT_TYPES = frozenset({"opened", "synchronize", "reopened"}) + +# For any other event, an unspecified `types:` means "whatever this event's +# defaults are", which varies per event and is not worth encoding. `*` collides +# with everything in the same bucket, so the check stays conservative: it may +# still over-report on an exotic event, never under-report. +ANY_TYPE = "*" + + def event_keys(workflow: dict) -> list[str]: - """The events that can start this workflow, keyed finely enough to spot a clash. + """The (event, branch, type) triples that can start this workflow. - `push` is keyed per branch, because a workflow on `push: staging` and one on - `push: main` do not collide. Everything else keys on the event name alone. + Keyed finely enough to spot a real clash and no finer: - `schedule`, `workflow_dispatch`, and `workflow_call` are deliberately absent: a - schedule or a manual run has no pipeline to belong to, and a `workflow_call` is - not an entry point at all. + - **branch**, because `push: staging` and `push: main` do not collide. + - **type**, because `pull_request: types: [closed]` and a workflow that runs + on `opened`/`synchronize` never run on the same event. A PR-close cleanup + workflow sitting beside the PR pipeline is the common shape, and it is not + a second run tree for the same trigger. + + `schedule`, `workflow_dispatch`, and `workflow_call` are deliberately absent: + a schedule or a manual run has no pipeline to belong to, and a + `workflow_call` is not an entry point at all. """ keys: list[str] = [] for event, config in triggers(workflow).items(): if event in ("schedule", "workflow_dispatch", "workflow_call"): continue - branches = None + branches, types = None, None if isinstance(config, dict): branches = config.get("branches") - if isinstance(branches, list) and branches: - keys.extend(f"{event}:{branch}" for branch in branches) + types = config.get("types") + + if isinstance(types, list) and types: + type_keys = [str(t) for t in types] + elif event in ("pull_request", "pull_request_target"): + type_keys = sorted(PULL_REQUEST_DEFAULT_TYPES) else: - keys.append(str(event)) + type_keys = [ANY_TYPE] + + branch_keys = ( + [str(b) for b in branches] if isinstance(branches, list) and branches else [ANY_TYPE] + ) + + for branch in branch_keys: + for type_key in type_keys: + suffix = "" if branch == ANY_TYPE else f":{branch}" + keys.append(f"{event}{suffix}#{type_key}") return keys +def describe_event(key: str) -> str: + """`push:main#*` -> ``push:main``; `pull_request#closed` -> ``pull_request (closed)``.""" + event, _, type_key = key.partition("#") + return event if type_key == ANY_TYPE else f"{event} ({type_key})" + + def check_run_trees( workflows: dict[str, dict], *, allow_external_reusables: bool = False ) -> tuple[list[str], list[str]]: @@ -413,20 +452,29 @@ def check_run_trees( for event in event_keys(workflow): by_event.setdefault(event, []).append(key) - for event, owners in sorted(by_event.items()): + # One finding per COLLIDING SET, not per event key. Keying by type means an + # overlapping pair now matches on `opened`, `synchronize` and `reopened` + # alike, and reporting that three times says nothing the first one did not. + collisions: dict[frozenset[str], list[str]] = {} + for event, owners in by_event.items(): if len(owners) > 1: - names = ", ".join(sorted(owners)) - errors.append( - f"`{event}` starts {len(owners)} workflows, so it produces {len(owners)} " - f"disconnected run trees: {names}\n" - f" Fold the side ones into the event's pipeline as `workflow_call` jobs, so one run " - f"shows everything that happened. A job that should not pay for itself on every run " - f"gates on a path check; a job behind an `environment:` approval needs an ungated plan " - f"job before it, since an approval blocks a job from starting.\n" - f" If a workflow genuinely has to keep its own run tree (a vendor OIDC claim bound to its " - f"filename, a release-train hook on someone else's event), add a `run-tree-ok: ` " - f"comment to it." - ) + collisions.setdefault(frozenset(owners), []).append(event) + + for owner_set, events in sorted(collisions.items(), key=lambda item: sorted(item[1])): + owners = sorted(owner_set) + names = ", ".join(owners) + triggers_shown = ", ".join(f"`{describe_event(event)}`" for event in sorted(events)) + errors.append( + f"{triggers_shown} starts {len(owners)} workflows, so it produces {len(owners)} " + f"disconnected run trees: {names}\n" + f" Fold the side ones into the event's pipeline as `workflow_call` jobs, so one run " + f"shows everything that happened. A job that should not pay for itself on every run " + f"gates on a path check; a job behind an `environment:` approval needs an ungated plan " + f"job before it, since an approval blocks a job from starting.\n" + f" If a workflow genuinely has to keep its own run tree (a vendor OIDC claim bound to its " + f"filename, a release-train hook on someone else's event), add a `run-tree-ok: ` " + f"comment to it." + ) if not allow_external_reusables: called = {callee for workflow in workflows.values() for _, callee in local_calls(workflow)} diff --git a/tests/test_branch_health.py b/tests/test_branch_health.py new file mode 100644 index 0000000..9cd5eb8 --- /dev/null +++ b/tests/test_branch_health.py @@ -0,0 +1,323 @@ +"""Unit tests for the red-branch sweep (``scripts/branch_health.py``). + +The sweep exists because of a real incident: cowork-server's staging publish +failed on the commit that fixed a broken release candidate, the failure was +deliberately silent because staging was not frozen, and the only thing the +engineering channel ever heard was the green *recovered* message from the re-run +three and a half hours later. Nobody was told the rc stream had stalled. + +The selection rules are the whole behaviour, so they are what these tests pin. In +particular the four that are easy to get backwards: staging is silent while it is +thawed no matter how red it is, the freeze window opening re-reports a branch that +has been red for days, a fresh failure is left to the pipeline's own notify job +rather than echoed, and the freeze workflow's runs are not on the staging branch. +""" + +import importlib.util +import json +import subprocess +from datetime import datetime, timedelta, timezone +from pathlib import Path + +_PATH = Path(__file__).resolve().parents[1] / "scripts" / "branch_health.py" +_spec = importlib.util.spec_from_file_location("branch_health", _PATH) +health = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(health) + +NOW = datetime(2026, 8, 14, 12, 0, tzinfo=timezone.utc) +CUTOFF = NOW - timedelta(minutes=90) +SETTLED = NOW - timedelta(minutes=30) + + +def run( + *, + path="publish-staging.yml", + name="Staging pre-release and publish to PyPI", + conclusion="failure", + minutes_ago=45, + run_id=1, +): + created = NOW - timedelta(minutes=minutes_ago) + return { + "id": run_id, + "path": path, + "name": name, + "conclusion": conclusion, + "created_at": created.isoformat().replace("+00:00", "Z"), + "head_sha": "abcdef1234567890", + "html_url": f"https://github.com/o/r/actions/runs/{run_id}", + "triggering_actor": {"login": "someone"}, + "display_title": "a commit", + } + + +def freeze_run(*, conclusion="success", minutes_ago=5): + created = NOW - timedelta(minutes=minutes_ago) + return {"conclusion": conclusion, "created_at": created.isoformat().replace("+00:00", "Z")} + + +def select(runs, *, branch="staging", frozen=True, freeze_runs=()): + return health.select_red( + runs, + branch=branch, + cutoff=CUTOFF, + settled=SETTLED, + frozen=frozen, + staging_branch="staging", + freeze_runs=freeze_runs, + ) + + +def completed(stdout="", stderr="", returncode=0): + return subprocess.CompletedProcess(args=[], returncode=returncode, stdout=stdout, stderr=stderr) + + +class TestNewestConclusivePerWorkflow: + def test_takes_the_newest_run_of_each_workflow(self): + runs = [ + run(run_id=1, conclusion="failure", minutes_ago=60), + run(run_id=2, conclusion="success", minutes_ago=40), + ] + assert [r["id"] for r in health.newest_conclusive_per_workflow(runs)] == [2] + + def test_ignores_cancelled_and_skipped(self): + """Neither is evidence about the branch, so neither may hide a failure.""" + runs = [ + run(run_id=1, conclusion="failure", minutes_ago=60), + run(run_id=2, conclusion="cancelled", minutes_ago=10), + run(run_id=3, conclusion="skipped", minutes_ago=5), + ] + assert [r["id"] for r in health.newest_conclusive_per_workflow(runs)] == [1] + + def test_startup_failure_counts_as_the_newest_run(self): + """It is CONCLUSIVE so it can supersede, but not RED so it is never reported. + + Dropping it from the ordering entirely made the sweep reach past it to an + older `failure` and report that as "the newest run is red", which is two + alerts for one broken branch: this sweep's and the startup sweep's. + """ + assert [r["id"] for r in health.newest_conclusive_per_workflow( + [run(conclusion="startup_failure")] + )] == [1] + + def test_a_startup_failure_supersedes_an_older_failure(self): + runs = [ + run(run_id=1, conclusion="failure", minutes_ago=60), + run(run_id=2, conclusion="startup_failure", minutes_ago=40), + ] + assert select(runs, frozen=True) == [], "the other sweep owns this one" + + def test_keyed_on_path_so_a_run_name_override_does_not_split_it(self): + runs = [ + run(run_id=1, name="Publish", conclusion="failure", minutes_ago=60), + run(run_id=2, name="Publish rc8 for #311", conclusion="success", minutes_ago=40), + ] + assert [r["id"] for r in health.newest_conclusive_per_workflow(runs)] == [2] + + def test_workflows_are_tracked_independently(self): + runs = [ + run(path="a.yml", run_id=1, conclusion="failure"), + run(path="b.yml", run_id=2, conclusion="success"), + ] + assert len(health.newest_conclusive_per_workflow(runs)) == 2 + + +class TestStagingIsSilentWhileThawed: + def test_red_staging_reports_nothing_outside_the_freeze(self): + """Mid-week staging is the integration branch; this is the whole policy.""" + assert select([run(conclusion="failure")], frozen=False) == [] + + def test_not_even_a_recovery_worth_of_noise(self): + """No red, no green, no grey while thawed.""" + assert select([run(conclusion="success")], frozen=False) == [] + assert select([run(conclusion="success")], frozen=True) == [] + + def test_a_freeze_that_just_opened_cannot_speak_for_a_thawed_branch(self): + """`frozen` is the authority; a stale freeze run must not override it.""" + assert select([run(conclusion="failure")], frozen=False, freeze_runs=[freeze_run()]) == [] + + def test_red_staging_reports_once_frozen(self): + assert len(select([run(conclusion="failure")], frozen=True)) == 1 + + +class TestBackstopNotEcho: + def test_a_fresh_failure_is_left_to_the_pipelines_own_notify_job(self): + """Repeating an alert seconds after it was sent trains people to ignore both.""" + assert select([run(conclusion="failure", minutes_ago=2)], branch="main", frozen=False) == [] + + def test_a_failure_that_has_stood_is_reported(self): + findings = select([run(conclusion="failure", minutes_ago=45)], branch="main", frozen=False) + assert findings[0]["reason"] == "still-red" + + def test_the_age_floor_does_not_apply_when_the_window_opens(self): + """The news there is the freeze, not the failure, so freshness is irrelevant.""" + runs = [run(conclusion="failure", minutes_ago=2)] + findings = select(runs, frozen=True, freeze_runs=[freeze_run()]) + assert findings[0]["reason"] == "freeze-opened" + + +class TestFreezeWindowOpening: + def test_a_branch_red_since_tuesday_is_reported_when_the_freeze_lands(self): + """The incident this exists for: stale red, freeze opens, nobody knows.""" + runs = [run(conclusion="failure", minutes_ago=60 * 72)] + findings = select(runs, frozen=True, freeze_runs=[freeze_run(minutes_ago=5)]) + assert [f["id"] for f in findings] == [1] + assert findings[0]["reason"] == "freeze-opened" + + def test_a_stale_red_is_not_reported_forever_after_the_window_opened(self): + """Without a fresh freeze run it falls back to the bounded lookback.""" + runs = [run(conclusion="failure", minutes_ago=60 * 72)] + assert select(runs, frozen=True, freeze_runs=[freeze_run(minutes_ago=60 * 24)]) == [] + + def test_a_failed_freeze_run_does_not_count_as_the_window_opening(self): + runs = [run(conclusion="failure", minutes_ago=60 * 72)] + assert select(runs, frozen=True, freeze_runs=[freeze_run(conclusion="failure")]) == [] + + def test_no_freeze_workflow_at_all_degrades_to_the_lookback(self): + """A repo off the release train still gets the ordinary bounded sweep.""" + assert len(select([run(conclusion="failure", minutes_ago=45)], frozen=True, freeze_runs=[])) == 1 + + +class TestMainAlwaysReports: + def test_main_reports_regardless_of_freeze_state(self): + """A red main is always worth interrupting for; the freeze is irrelevant.""" + assert len(select([run(conclusion="failure")], branch="main", frozen=False)) == 1 + + def test_main_respects_the_lookback_so_it_does_not_repeat_forever(self): + assert select([run(conclusion="failure", minutes_ago=200)], branch="main", frozen=False) == [] + + def test_timed_out_counts_as_red(self): + assert len(select([run(conclusion="timed_out")], branch="main", frozen=False)) == 1 + + +class TestFetchFreezeRuns: + """A scheduled run is attributed to the DEFAULT branch, never to staging. + + Reading the freeze workflow out of a `staging` run listing finds nothing, + forever, silently, which would have made the freeze-opened trigger dead code. + """ + + def test_finds_the_freeze_workflow_by_name_and_reads_its_own_runs(self): + workflows = json.dumps( + {"workflows": [{"id": 3, "name": "Tests"}, {"id": 8, "name": health.FREEZE_WORKFLOW_NAME}]} + ) + runs = json.dumps({"workflow_runs": [{"conclusion": "success", "created_at": "2026-08-14T11:55:00Z"}]}) + calls = [] + + def runner(argv): + calls.append(argv[-1]) + return completed(workflows if "actions/workflows?" in argv[-1] else runs) + + assert len(health.fetch_freeze_runs("o/r", runner=runner)) == 1 + assert "actions/workflows/8/runs" in calls[1] + assert "branch=" not in calls[1], "the freeze runs are not on the staging branch" + + def test_no_freeze_workflow_returns_empty_rather_than_raising(self): + runner = lambda argv: completed(json.dumps({"workflows": [{"id": 3, "name": "Tests"}]})) + assert health.fetch_freeze_runs("o/r", runner=runner) == [] + + +class TestAgeIsMeasuredFromTheAttempt: + """`created_at` stays pinned to attempt 1, so a re-run must not be judged by it. + + Measured against live runs: a cowork-server attempt 2 carried `created_at` + 22:26:37 and `run_started_at` 23:47:11. Eighty minutes apart, so a 90-minute + lookback had already almost expired before the attempt began, and the re-run + this sweep exists to catch was filtered out as too old. + """ + + def test_a_rerun_is_judged_by_when_the_attempt_started(self): + stale = run(conclusion="failure", minutes_ago=60 * 40) + stale["run_started_at"] = (NOW - timedelta(minutes=45)).isoformat().replace("+00:00", "Z") + findings = select([stale], branch="main", frozen=False) + assert [f["id"] for f in findings] == [1], "the attempt began inside the window" + + def test_created_at_alone_would_have_dropped_it(self): + """Same run, proving the old bound is what excluded it.""" + stale = run(conclusion="failure", minutes_ago=60 * 40) + assert health.parse_time(stale["created_at"]) < CUTOFF + assert select([stale], branch="main", frozen=False) == [] + + def test_falls_back_to_created_at_when_the_attempt_start_is_absent(self): + """The freeze-run listing is trimmed to the fields it needs.""" + assert len(select([run(conclusion="failure", minutes_ago=45)], branch="main", frozen=False)) == 1 + + def test_newest_is_decided_by_attempt_start_too(self): + old_rerun = run(run_id=1, conclusion="failure", minutes_ago=90) + old_rerun["run_started_at"] = (NOW - timedelta(minutes=5)).isoformat().replace("+00:00", "Z") + fresh = run(run_id=2, conclusion="success", minutes_ago=40) + assert [r["id"] for r in health.newest_conclusive_per_workflow([old_rerun, fresh])] == [1] + + +class TestScope: + def test_reports_every_workflow_by_default(self): + runs = [run(path="a.yml", run_id=1), run(path="b.yml", run_id=2)] + assert len(select(runs, branch="main", frozen=False)) == 2 + + def test_an_allowlist_narrows_it(self): + runs = [run(path="a.yml", run_id=1), run(path="b.yml", run_id=2)] + findings = health.select_red( + runs, branch="main", cutoff=CUTOFF, settled=SETTLED, frozen=False, + staging_branch="staging", only=["a.yml"], + ) + assert [f["path"] for f in findings] == ["a.yml"] + + def test_the_sweep_never_reports_itself(self): + """A watchdog whose own run went red would otherwise alert on itself.""" + runs = [run(path=".github/workflows/pipeline-watchdog.yml", run_id=1)] + findings = health.select_red( + runs, branch="main", cutoff=CUTOFF, settled=SETTLED, frozen=False, + staging_branch="staging", exclude=[".github/workflows/pipeline-watchdog.yml"], + ) + assert findings == [] + + def test_exclusion_wins_over_an_allowlist(self): + path = ".github/workflows/pipeline-watchdog.yml" + findings = health.select_red( + [run(path=path)], branch="main", cutoff=CUTOFF, settled=SETTLED, frozen=False, + staging_branch="staging", only=[path], exclude=[path], + ) + assert findings == [] + + +class TestUnreadableBranchIsNotAnAllClear: + def test_a_branch_whose_history_fails_is_reported_as_unreadable(self, tmp_path, monkeypatch): + """The summary used to print "No deploy branch is sitting red" here.""" + out = tmp_path / "gh-output" + monkeypatch.setenv("GITHUB_OUTPUT", str(out)) + runner = lambda argv: completed(stderr="HTTP 403", returncode=1) + code = health.main( + ["--repo", "o/r", "--branches", "main", "--out", str(tmp_path / "red.json")], + runner=runner, + now=NOW, + ) + assert code == 0, "a watchdog must not redden the repo over a transient API error" + written = dict(line.split("=", 1) for line in out.read_text().strip().splitlines()) + assert written["count"] == "0" + assert written["unreadable"] == "main" + + def test_a_readable_branch_reports_no_unreadable(self, tmp_path, monkeypatch): + out = tmp_path / "gh-output" + monkeypatch.setenv("GITHUB_OUTPUT", str(out)) + runner = lambda argv: completed(json.dumps({"workflow_runs": []})) + health.main( + ["--repo", "o/r", "--branches", "main", "--out", str(tmp_path / "red.json")], + runner=runner, + now=NOW, + ) + written = dict(line.split("=", 1) for line in out.read_text().strip().splitlines()) + assert written["unreadable"] == "" + + +class TestFindingShape: + def test_carries_what_the_message_needs(self): + finding = select([run(conclusion="failure")], frozen=True)[0] + assert finding["branch"] == "staging" + assert finding["actor"] == "someone" + assert finding["head_sha"].startswith("abcdef") + assert finding["html_url"].endswith("/1") + + def test_missing_triggering_actor_does_not_crash(self): + raw = run(conclusion="failure") + raw["triggering_actor"] = None + assert select([raw], frozen=True)[0]["actor"] == "unknown" diff --git a/tests/test_freeze_state.py b/tests/test_freeze_state.py new file mode 100644 index 0000000..d72ffe4 --- /dev/null +++ b/tests/test_freeze_state.py @@ -0,0 +1,183 @@ +"""Unit tests for the release-freeze contract (``scripts/freeze_state.py``). + +The behaviour worth pinning is the asymmetry: a reader that cannot establish the +freeze state must escalate, and a writer that cannot apply the freeze must fail. +Getting either backwards is silent in production. An escalating writer would +report a freeze that never happened; a failing reader would redden a green +pipeline from its notify job. +""" + +import importlib.util +import json +import subprocess +from pathlib import Path + +import pytest + +_PATH = Path(__file__).resolve().parents[1] / "scripts" / "freeze_state.py" +_spec = importlib.util.spec_from_file_location("freeze_state", _PATH) +freeze_state = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(freeze_state) + + +def completed(stdout="", stderr="", returncode=0): + return subprocess.CompletedProcess(args=[], returncode=returncode, stdout=stdout, stderr=stderr) + + +def fake_runner(responses): + """Return a runner that answers each `gh api` call from `responses` in order.""" + calls = [] + + def runner(argv): + calls.append(list(argv)) + return responses[len(calls) - 1] + + runner.calls = calls + return runner + + +RULESET_LIST = json.dumps( + [ + {"id": 7, "name": "staging-freeze", "enforcement": "disabled"}, + {"id": 9, "name": "something-else", "enforcement": "active"}, + ] +) + +RULESET_DETAIL = json.dumps( + { + "id": 7, + "name": "staging-freeze", + "target": "branch", + "enforcement": "disabled", + "bypass_actors": [{"actor_id": 1, "actor_type": "Integration"}], + "conditions": {"ref_name": {"include": ["refs/heads/staging"]}}, + "rules": [{"type": "update"}], + } +) + + +class TestIsFrozen: + def test_active_enforcement_is_frozen(self): + listing = json.dumps([{"id": 7, "name": "staging-freeze", "enforcement": "active"}]) + assert freeze_state.is_frozen("o/r", "staging-freeze", runner=fake_runner([completed(listing)])) + + def test_disabled_enforcement_is_not_frozen(self): + assert not freeze_state.is_frozen( + "o/r", "staging-freeze", runner=fake_runner([completed(RULESET_LIST)]) + ) + + def test_matches_on_name_not_position(self): + """A repo carries several rulesets; only the named one decides the freeze.""" + runner = fake_runner([completed(RULESET_LIST)]) + assert freeze_state.is_frozen("o/r", "something-else", runner=runner) + + def test_missing_ruleset_raises(self): + with pytest.raises(freeze_state.LookupError_, match="not found"): + freeze_state.is_frozen("o/r", "absent", runner=fake_runner([completed(RULESET_LIST)])) + + def test_api_failure_raises(self): + runner = fake_runner([completed(stderr="HTTP 403", returncode=1)]) + with pytest.raises(freeze_state.LookupError_, match="Could not read rulesets"): + freeze_state.is_frozen("o/r", "staging-freeze", runner=runner) + + +class TestReadMode: + """The alerting path. It must never fail the job and never under-report.""" + + def test_reports_false_when_thawed(self, capsys): + code = freeze_state.main( + ["read", "--repo", "o/r"], runner=fake_runner([completed(RULESET_LIST)]) + ) + assert code == 0 + assert "frozen=false" in capsys.readouterr().out + + def test_unreadable_ruleset_escalates_to_frozen(self, capsys): + """A lookup problem must not silence a real release-blocking failure.""" + runner = fake_runner([completed(stderr="HTTP 403", returncode=1)]) + code = freeze_state.main(["read", "--repo", "o/r"], runner=runner) + out = capsys.readouterr().out + assert code == 0, "a notify job must never redden a green run" + assert "frozen=true" in out + assert "::warning::" in out + + def test_missing_ruleset_escalates_to_frozen(self, capsys): + runner = fake_runner([completed(RULESET_LIST)]) + code = freeze_state.main(["read", "--repo", "o/r", "--ruleset-name", "renamed"], runner=runner) + assert code == 0 + assert "frozen=true" in capsys.readouterr().out + + def test_on_error_fail_is_available_for_non_alerting_callers(self): + runner = fake_runner([completed(stderr="HTTP 403", returncode=1)]) + assert freeze_state.main(["read", "--repo", "o/r", "--on-error", "fail"], runner=runner) == 1 + + def test_writes_github_output(self, tmp_path, monkeypatch): + out = tmp_path / "gh-output" + monkeypatch.setenv("GITHUB_OUTPUT", str(out)) + freeze_state.main(["read", "--repo", "o/r"], runner=fake_runner([completed(RULESET_LIST)])) + assert out.read_text().strip() == "frozen=false" + + +class TestSetMode: + """The freeze/unfreeze path. It must preserve the ruleset and fail loudly.""" + + def test_put_preserves_bypass_actors_conditions_and_rules(self, tmp_path): + body = tmp_path / "ruleset.json" + runner = fake_runner([completed(RULESET_LIST), completed(RULESET_DETAIL), completed("{}")]) + freeze_state.set_enforcement( + "o/r", "staging-freeze", "active", body_path=str(body), runner=runner + ) + written = json.loads(body.read_text()) + assert written["enforcement"] == "active" + assert written["bypass_actors"] == [{"actor_id": 1, "actor_type": "Integration"}] + assert written["rules"] == [{"type": "update"}] + assert written["conditions"]["ref_name"]["include"] == ["refs/heads/staging"] + + def test_reads_the_detail_endpoint_not_the_listing(self, tmp_path): + """The listing omits rules and bypass actors; PUTting it back drops them.""" + runner = fake_runner([completed(RULESET_LIST), completed(RULESET_DETAIL), completed("{}")]) + freeze_state.set_enforcement( + "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner + ) + assert runner.calls[1] == ["gh", "api", "repos/o/r/rulesets/7"] + + def test_failed_put_raises(self, tmp_path): + runner = fake_runner( + [completed(RULESET_LIST), completed(RULESET_DETAIL), completed(stderr="HTTP 422", returncode=1)] + ) + with pytest.raises(freeze_state.LookupError_, match="Could not update"): + freeze_state.set_enforcement( + "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner + ) + + def test_a_non_json_detail_body_is_a_lookup_error_not_a_traceback(self, tmp_path): + """The one unguarded parse in the module. An operator paged by a stopped + release train reads `main()`'s "provisioning has drifted" annotation, not a + JSONDecodeError stack.""" + runner = fake_runner([completed(RULESET_LIST), completed("502")]) + with pytest.raises(freeze_state.LookupError_, match="was not JSON"): + freeze_state.set_enforcement( + "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner + ) + + def test_an_error_body_on_stdout_still_reaches_the_message(self, tmp_path): + """`gh` puts the API's error body on stdout and its own noise on stderr, so + taking stderr alone can print an error with no cause in it.""" + runner = fake_runner( + [ + completed(RULESET_LIST), + completed(RULESET_DETAIL), + completed(stdout='{"message":"Resource not accessible by integration"}', returncode=1), + ] + ) + with pytest.raises(freeze_state.LookupError_, match="not accessible by integration"): + freeze_state.set_enforcement( + "o/r", "staging-freeze", "active", body_path=str(tmp_path / "b.json"), runner=runner + ) + + def test_missing_ruleset_fails_the_job(self, tmp_path): + """Never escalate here: a freeze that did not apply must stop the train.""" + code = freeze_state.main( + ["set", "--repo", "o/r", "--enforcement", "active", "--body-path", str(tmp_path / "b.json")], + runner=fake_runner([completed("[]")]), + ) + assert code == 1 diff --git a/tests/test_notify_decision.py b/tests/test_notify_decision.py new file mode 100644 index 0000000..e58f72c --- /dev/null +++ b/tests/test_notify_decision.py @@ -0,0 +1,169 @@ +"""Unit tests for the alert decision (``scripts/notify_decision.py``). + +This logic lived in two `if:` expressions and a shell `if/elif/else` inside +`notify-pipeline-status/action.yml`, where nothing could reach it. The defect that +survived there is the one pinned first below: a manual run of any release-train +wrapper posted a green ``Recovered`` for a failure that had never happened, +because the composite inferred "this is the smoke test" from +``github.event_name == 'workflow_dispatch'`` and a called workflow inherits the +CALLER's event. + +The policy these pin, in the order it is easy to get wrong: + +- staging outside the freeze window is silent in BOTH directions +- a green run whose predecessor was also green posts nothing +- a failure always posts, and the freeze veto is the only thing that may stop it +- an unreadable run history reads as "not a recovery", never as an error +""" + +import importlib.util +from pathlib import Path + +import pytest + +_PATH = Path(__file__).resolve().parents[1] / "scripts" / "notify_decision.py" +_spec = importlib.util.spec_from_file_location("notify_decision", _PATH) +notify = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(notify) + + +def decide(**kwargs): + """`decide` with the defaults a caller that passes nothing would get.""" + return notify.decide( + **{ + "status": "failed", + "freeze_scoped": "false", + "frozen": "", + "force_post": "false", + "prev_conclusion": "", + **kwargs, + } + ) + + +class TestTheDispatchEscapeHatch: + """The defect this split exists to fix.""" + + def test_a_green_run_does_not_post_just_because_a_human_started_it(self): + """The regression: a manual "Staging Freeze" reported a recovery from nothing. + + The four release-train reusables pass no `force-post`, and all their + wrappers declare `workflow_dispatch`. Nothing about the event may reach + this decision. + """ + assert decide(status="recovered", prev_conclusion="success")["post"] == "false" + + def test_the_smoke_test_still_posts_on_demand(self): + """`force-post` is how the notify workflow's own dispatch says so.""" + assert decide(status="recovered", force_post="true")["post"] == "true" + + def test_force_post_needs_no_evidence_at_all(self): + assert decide(status="recovered", force_post="true", prev_conclusion="")["post"] == "true" + + def test_force_post_does_not_override_the_freeze_veto(self): + """Silence outside the window is the stronger rule; a smoke test is not an + excuse to page the channel about a thawed staging branch.""" + d = decide(status="recovered", force_post="true", freeze_scoped="true", frozen="false") + assert d["post"] == "false" + assert d["level"] == "silenced" + + +class TestRecoveryNeedsAPriorFailure: + @pytest.mark.parametrize("conclusion", ["failure", "timed_out", "startup_failure"]) + def test_posts_when_the_previous_run_failed(self, conclusion): + assert decide(status="recovered", prev_conclusion=conclusion)["post"] == "true" + + @pytest.mark.parametrize("conclusion", ["success", "cancelled", "skipped", ""]) + def test_stays_quiet_otherwise(self, conclusion): + """`cancelled` and `skipped` are not evidence anything was broken, and an + empty conclusion means the lookup was refused or never made.""" + assert decide(status="recovered", prev_conclusion=conclusion)["post"] == "false" + + def test_an_unreadable_history_is_not_a_recovery_and_not_an_error(self): + """A missing `actions: read` grant must degrade to silence. A notify step + that fails here would turn a green pipeline red.""" + assert decide(status="recovered", prev_conclusion="")["post"] == "false" + + +class TestFailuresAlwaysPost: + def test_a_failure_posts_with_no_history_lookup_at_all(self): + assert decide(status="failed")["post"] == "true" + + def test_a_failure_posts_while_the_freeze_window_is_open(self): + assert decide(status="failed", freeze_scoped="true", frozen="true")["post"] == "true" + + def test_the_freeze_veto_is_the_only_thing_that_silences_a_failure(self): + assert decide(status="failed", freeze_scoped="true", frozen="false")["post"] == "false" + + +class TestFreezeScoping: + def test_silent_in_both_directions_while_thawed(self): + """The whole policy: half a story is worse than none.""" + assert decide(status="failed", freeze_scoped="true", frozen="false")["post"] == "false" + assert decide(status="recovered", freeze_scoped="true", frozen="false", + prev_conclusion="failure")["post"] == "false" + + def test_both_directions_post_while_frozen(self): + assert decide(status="failed", freeze_scoped="true", frozen="true")["post"] == "true" + assert decide(status="recovered", freeze_scoped="true", frozen="true", + prev_conclusion="failure")["post"] == "true" + + def test_an_unscoped_caller_is_never_silenced_by_an_empty_frozen(self): + """`frozen` is '' when the freeze step did not run, which is NOT 'false'. + Collapsing the two would silence every prod and main caller.""" + assert decide(status="failed", freeze_scoped="false", frozen="")["post"] == "true" + + def test_an_unestablished_freeze_state_escalates(self): + """freeze_state.py reports frozen=true when it cannot read the ruleset, so + the failure still escalates. Being wrong this way costs one alert.""" + assert decide(status="failed", freeze_scoped="true", frozen="true")["post"] == "true" + + +class TestStyling: + def test_a_failure_is_red_and_shouts(self): + d = decide(status="failed") + assert (d["level"], d["color"], d["prefix"], d["verb"]) == ("alert", "#FF4444", "FAILED", "failed") + + def test_a_recovery_is_green(self): + d = decide(status="recovered", prev_conclusion="failure") + assert (d["level"], d["color"], d["prefix"], d["verb"]) == ( + "recovered", "#00C851", "Recovered", "recovered", + ) + + def test_a_silenced_decision_carries_no_styling_to_leak(self): + d = decide(status="failed", freeze_scoped="true", frozen="false") + assert (d["color"], d["icon"], d["verb"], d["prefix"]) == ("", "", "", "") + + def test_every_outcome_emits_the_same_keys(self): + """The Slack payload interpolates all of them, so a missing one renders as + an empty string in the message rather than failing loudly.""" + expected = {"post", "level", "color", "icon", "verb", "prefix"} + assert set(decide(status="failed")) == expected + assert set(decide(status="recovered")) == expected + assert set(decide(status="failed", freeze_scoped="true", frozen="false")) == expected + + +class TestMainWritesTheStepOutputs: + def test_writes_every_key_to_github_output(self, tmp_path, monkeypatch): + out = tmp_path / "gh-output" + monkeypatch.setenv("GITHUB_OUTPUT", str(out)) + assert notify.main(["--status", "failed"]) == 0 + written = dict(line.split("=", 1) for line in out.read_text().strip().splitlines()) + assert written["post"] == "true" + assert written["level"] == "alert" + assert written["prefix"] == "FAILED" + + def test_survives_no_github_output(self): + """Runs locally and in `act` without a step-output file.""" + assert notify.main(["--status", "recovered"]) == 0 + + def test_explains_itself_in_the_log(self, capsys): + notify.main(["--status", "recovered", "--prev-conclusion", "success"]) + assert "ordinary green run" in capsys.readouterr().out + + def test_the_defaults_match_the_composite_action_inputs(self): + """`notify-pipeline-status/action.yml` defaults status to `failed`, + freeze-scoped and force-post to "false", and passes an empty + prev-conclusion when the lookup step was skipped.""" + assert notify.main([]) == 0 + assert decide()["level"] == "alert" diff --git a/tests/test_workflow_graph.py b/tests/test_workflow_graph.py index 15213cd..139d6ab 100644 --- a/tests/test_workflow_graph.py +++ b/tests/test_workflow_graph.py @@ -365,12 +365,23 @@ def test_inheriting_in_the_callee_is_what_resolves_it(self, tmp_path): class TestEventKeys: def test_push_keys_per_branch(self): assert gate.event_keys({"on": {"push": {"branches": ["main", "staging"]}}}) == [ - "push:main", - "push:staging", + "push:main#*", + "push:staging#*", ] def test_push_with_no_branch_filter_keys_on_the_event(self): - assert gate.event_keys({"on": {"push": None}}) == ["push"] + assert gate.event_keys({"on": {"push": None}}) == ["push#*"] + + def test_pull_request_expands_to_its_declared_types(self): + assert gate.event_keys({"on": {"pull_request": {"types": ["closed"]}}}) == ["pull_request#closed"] + + def test_unspecified_pull_request_types_are_githubs_defaults_not_everything(self): + """The false positive this fixes: `closed` and the defaults are disjoint.""" + assert gate.event_keys({"on": {"pull_request": None}}) == [ + "pull_request#opened", + "pull_request#reopened", + "pull_request#synchronize", + ] def test_schedule_and_dispatch_are_exempt(self): """They have no pipeline to belong to, which is why they get their own run.""" @@ -379,9 +390,23 @@ def test_schedule_and_dispatch_are_exempt(self): def test_workflow_call_is_not_an_event_key(self): assert gate.event_keys({"on": {"workflow_call": None}}) == [] - def test_pull_request_keys_on_the_event_not_the_base(self): - """A base-branch filter does not separate run trees the way a push branch does.""" - assert gate.event_keys({"on": {"pull_request": {"types": ["opened"]}}}) == ["pull_request"] + def test_a_declared_pull_request_type_becomes_the_only_key(self): + assert gate.event_keys({"on": {"pull_request": {"types": ["opened"]}}}) == ["pull_request#opened"] + + def test_a_base_branch_filter_narrows_the_key(self): + """Named for what the code does. The old name here claimed `pull_request` + keys on the event and not the base, and the code has never done that: the + branch loop applies to every event, not only `push`. The input had no + `branches:` at all, so it passed either way and pinned nothing. + + The consequence is a real (and pre-existing) blind spot: a workflow filtered + to `main` and one with no filter both fire on a PR to `main` and do not + collide, because a filter is not treated as a subset of no filter. + """ + assert gate.event_keys({"on": {"pull_request": {"branches": ["main"], "types": ["opened"]}}}) == [ + "pull_request:main#opened" + ] + assert gate.event_keys({"on": {"pull_request": {"types": ["opened"]}}}) == ["pull_request#opened"] class TestRunTrees: @@ -400,6 +425,46 @@ def test_different_branches_do_not_collide(self, tmp_path): errors, _ = gate.check_run_trees(gate.load_workflows(tmp_path)) assert errors == [] + def test_a_pr_close_cleanup_does_not_collide_with_the_pr_pipeline(self, tmp_path): + """Disjoint `types` never co-fire, so they are not two trees for one event. + + This shape (a cleanup on PR close beside the PR pipeline) was the majority + of what this check reported, and the only way to silence it was a + `run-tree-ok` comment — which is how a real finding later gets waved + through on the same file. + """ + write( + tmp_path, + "pipeline.yml", + {"on": {"pull_request": {"types": ["opened", "synchronize"]}}, "jobs": {"a": {"runs-on": "x"}}}, + ) + write( + tmp_path, + "cleanup.yml", + {"on": {"pull_request": {"types": ["closed"]}}, "jobs": {"b": {"runs-on": "x"}}}, + ) + errors, _ = gate.check_run_trees(gate.load_workflows(tmp_path)) + assert errors == [] + + def test_an_unspecified_pr_trigger_still_collides_with_an_overlapping_one(self, tmp_path): + """Declaring nothing is not a licence to share the event with `opened`.""" + write(tmp_path, "tests.yml", {"on": {"pull_request": None}, "jobs": {"a": {"runs-on": "x"}}}) + write( + tmp_path, + "scan.yml", + {"on": {"pull_request": {"types": ["opened", "labeled"]}}, "jobs": {"b": {"runs-on": "x"}}}, + ) + errors, _ = gate.check_run_trees(gate.load_workflows(tmp_path)) + assert len(errors) == 1 + assert "tests.yml" in errors[0] and "scan.yml" in errors[0] + + def test_an_overlap_on_several_types_is_reported_once(self, tmp_path): + """Three shared types said three times says nothing the first one did not.""" + write(tmp_path, "a.yml", {"on": {"pull_request": None}, "jobs": {"a": {"runs-on": "x"}}}) + write(tmp_path, "b.yml", {"on": {"pull_request": None}, "jobs": {"b": {"runs-on": "x"}}}) + errors, _ = gate.check_run_trees(gate.load_workflows(tmp_path)) + assert len(errors) == 1 + def test_two_schedules_do_not_collide(self, tmp_path): write(tmp_path, "nightly.yml", {"on": {"schedule": [{"cron": "0 5 * * *"}]}, "jobs": {"a": {"runs-on": "x"}}}) write(tmp_path, "weekly.yml", {"on": {"schedule": [{"cron": "0 5 * * 1"}]}, "jobs": {"b": {"runs-on": "x"}}})