From 0f0875c98ef84abc21066a15792988127f429eb2 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sat, 22 Aug 2026 22:50:15 +0200 Subject: [PATCH] feat(kanban): a bug-labelled issue lands in Ready, not Backlog (backend#2348) The org rule "bugs get `work-type:bug` and go straight into `Ready` (defects skip refinement)" was documented in CLAUDE.md, org-standards.md and every repo's contributor guidance, and implemented nowhere. `add-to-kanban.yml` adds every new issue at `Backlog`; no workflow read the label afterwards. Measured 2026-08-22: seven bug-labelled tickets filed in one day (backend#2324, #2327, #2329, #2340, #2341, #2344, frontend-app#871) all landed in `Backlog` and needed a hand-run `updateProjectV2ItemFieldValue`. A 100% miss rate is the tell that nothing does it at all, rather than that people forget sometimes. A NEW REUSABLE, not a line in `add-to-kanban.yml`. That file is the org's only per-repo COPY, byte-compared against this repo's by `caller-drift.py`, so an edit here makes all 18 other copies read DRIFTED on the next Monday audit -- and the documented escape (merge every other repo BEFORE .github) is impossible for a change that must be authored here first. A reusable resolves from `@main` at run time, so there is no byte comparison and no ordering trap. It also keeps `add-to-kanban.yml` free for backend#1877 and backend#2157, which both have it in their path. MONOTONIC BY CONSTRUCTION. It promotes only from `Backlog` or no-Status, and it refuses unless the live board reports `Ready` strictly AFTER `Backlog` -- so a reordered board makes the "promotion" a demotion and the workflow says so rather than performing it. Positions come from the board's own option order, never from a rank table held here. A card at `In progress`, `On dev`, `Prod`, `Done` or `Cancelled` is left where it is. Ships with ZERO callers, deliberately, for exactly one PR: the decision surface is reviewed on its own and the callers follow, which is the two-step `stale_backlog_migration_in_flight` and `blocked_gate_rollout_pending` already document. Every repo's inventory row says so in writing. Evidence: `make check` green (actionlint 0, shellcheck clean, house-rules clean, action-pins clean, mint-scope clean -- the new mint is scoped, not a twelfth unscoped one). 37 new assertions, 8 mutations all caught with their anchors asserted. `caller-drift.py` against the live org: 0 findings mentioning bug-to-ready.yml. `kanban-columns-check.py` against the live board now collects `Backlog` and `Ready` from this workflow and confirms both exist. Co-Authored-By: Claude Opus 5 --- .github/workflows/bug-to-ready.yml | 347 +++++++++++++++++++++++++ .github/workflows/kanban-columns.yml | 1 + Makefile | 12 +- repo-inventory.yml | 53 ++++ scripts/kanban-columns-check.py | 6 + scripts/tests/bug-to-ready-selftest.py | 331 +++++++++++++++++++++++ 6 files changed, 749 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/bug-to-ready.yml create mode 100644 scripts/tests/bug-to-ready-selftest.py diff --git a/.github/workflows/bug-to-ready.yml b/.github/workflows/bug-to-ready.yml new file mode 100644 index 0000000..7fa29ee --- /dev/null +++ b/.github/workflows/bug-to-ready.yml @@ -0,0 +1,347 @@ +name: Bug label lands the card in Ready + +# THE RULE THIS IMPLEMENTS, and nothing did (backend#2348). +# +# "bugs get `work-type:bug` and go straight into `Ready` (defects skip +# refinement)" +# +# It is written in CLAUDE.md, in org-standards.md and in every repo's contributor +# guidance, and it was enforced entirely by whoever remembered. `add-to-kanban.yml` +# puts every new issue in `Backlog` and no workflow read the label afterwards. +# +# MEASURED 2026-08-22: seven bug-labelled tickets filed in one day -- backend#2324, +# #2327, #2329, #2340, #2341, #2344 and frontend-app#871 -- and all seven landed in +# `Backlog` and needed a hand-run `updateProjectV2ItemFieldValue`. A 100% miss rate +# is the tell that nothing does it at all, rather than that people forget sometimes: +# a rule people mostly follow produces a mixed record. +# +# WHY IT IS NOT TIDINESS. `Ready` is the queue engineers pull from and `Backlog` is +# the refinement queue nobody pulls from -- that split is the whole point. A defect +# filed correctly and labelled correctly was, by default, invisible to the people +# meant to pick it up. The release train's deferral story rides on the same path: it +# files a ticket for every Medium/Low it ships as "a record rather than a dismissal", +# and those landed unread too. +# +# WHY A NEW REUSABLE RATHER THAN A LINE IN `add-to-kanban.yml` +# ----------------------------------------------------------- +# `add-to-kanban.yml` is the org's ONLY per-repo COPY (`copies:` in +# repo-inventory.yml, a list of exactly one), and `caller-drift.py` compares each +# repo's copy against this repo's byte for byte. So an edit here is not an edit to +# one file: it makes all eighteen other copies read DRIFTED on the next Monday +# audit, and the documented way out is to merge every other repo BEFORE .github -- +# which is impossible for a change that has to be authored in .github first. A +# reusable is resolved at run time from `@main`, so the same fix reaches the fleet +# with no byte-comparison to satisfy and no ordering trap. See the PR body. +# +# It also keeps two concerns apart. `add-to-kanban.yml` answers "is this on the +# board"; this answers "which column does the label imply". backend#1877 and +# backend#2157 both have that file in their path, and a second concern welded into +# it is a second concern they have to carry. + +on: + workflow_call: + inputs: + project-number: + type: number + default: 2 + org: + type: string + default: tracebloc + # The label is an input because a repo could conceivably spell it + # differently; the two COLUMN names are not, because they are the rule + # rather than a setting. A repo that could configure `Ready` away would be + # a repo where this workflow reports success while doing nothing. + bug-label: + type: string + default: work-type:bug + +# The Bug template applies the label at creation, so `opened` and `labeled` can +# both fire for the same issue seconds apart. Both runs would compute the same +# write, so the race is harmless -- but serialising them keeps the run log honest +# about what happened. NEVER cancel-in-progress: a cancelled run is a card that +# silently stayed in Backlog, which is this ticket. +concurrency: + group: bug-to-ready-${{ github.repository }}-${{ github.event.issue.number }} + cancel-in-progress: false + +jobs: + bug-to-ready: + runs-on: ubuntu-latest + timeout-minutes: 10 + # NO GITHUB_TOKEN AT ALL, same reasoning as add-to-kanban.yml (#2181): every + # call below authenticates as the App, so the workflow token needs nothing, + # and an empty grant is the only version of that claim a reader can check. + permissions: {} + steps: + # ------------------------------------------------------------------ + # Is this event a candidate at all? Answered BEFORE any credential is + # minted, so the overwhelmingly common no-op (any non-bug issue event) + # costs no token. + # ------------------------------------------------------------------ + - name: Is this a bug-labelled issue? + id: candidate + env: + EVENT_NAME: ${{ github.event_name }} + # PRESENT ONLY WHEN THE "ISSUE" IS A PULL REQUEST. `issues:` events + # never fire for PRs, so this is belt to the caller's braces -- but a + # reusable may be called from any trigger, and `issue_comment` DOES + # fire for both. Per the board's model an issue occupies + # Backlog/North Stars/Ready only; the flow columns are for PRs, so a + # PR reaching this logic would be a demotion waiting to happen. + IS_PULL_REQUEST: ${{ github.event.issue.pull_request != null }} + LABELS_JSON: ${{ toJSON(github.event.issue.labels) }} + BUG_LABEL: ${{ inputs.bug-label }} + run: | + set -euo pipefail + # selftest:candidate-start + # yes -> a bug-labelled issue; go on to the board + # no: -> legitimately nothing to do, green + # error: -> CANNOT TELL, which is a finding and not a + # pass (backend#1729 rule 3). A payload whose + # labels will not parse is exactly how this + # check would come to pass forever. + candidate_verdict() { + if [ "${EVENT_NAME:-}" != "issues" ]; then + echo "no:not-an-issues-event"; return + fi + if [ "${IS_PULL_REQUEST:-false}" = "true" ]; then + echo "no:pull-request"; return + fi + if [ -z "${BUG_LABEL:-}" ]; then + echo "error:no-label-configured"; return + fi + # Two jq calls' worth of care, not one. `jq -e 'any(...)'` folds "no + # match" and "unparseable" into the same non-zero exit, which is the + # fail-open this org keeps finding: a malformed payload would read as + # "not a bug" and the card would stay in Backlog behind a green check. + _n=$(printf '%s' "${LABELS_JSON:-}" \ + | jq -r --arg l "$BUG_LABEL" '[.[] | select(.name == $l)] | length') \ + || { echo "error:labels-unreadable"; return; } + case "$_n" in + ''|*[!0-9]*) echo "error:labels-unreadable"; return ;; + esac + if [ "$_n" -eq 0 ]; then echo "no:label-absent"; return; fi + echo yes + } + VERDICT=$(candidate_verdict) + case "$VERDICT" in + error:*) + echo "::error::cannot tell whether this issue carries the bug label ($VERDICT)." \ + "Refusing to report success from a read this workflow did not understand." + exit 1 ;; + no:*) + echo "::notice::not a candidate ($VERDICT) — nothing to do" ;; + esac + # selftest:candidate-end + echo "verdict=$VERDICT" >> "$GITHUB_OUTPUT" + + # Board writes authenticate as the tracebloc-release-train App (backend#2036), + # never a human's PAT. `owner:` yields an ORG-scoped installation token; a + # repo-scoped one cannot write an org ProjectV2. No fallback: a fallback would + # let a broken App path keep looking like a working one. + - name: Mint an installation token + id: app-token + if: steps.candidate.outputs.verdict == 'yes' + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ secrets.RELEASE_TRAIN_APP_ID }} + private-key: ${{ secrets.RELEASE_TRAIN_APP_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + # LEAST PRIVILEGE, DERIVED FROM WHAT THE STEP BELOW RUNS (backend#2157). + # It resolves ONE issue node in the CALLING repo and reads+writes the org + # project's Status field. That is `issues: read` plus + # `organization-projects: write`, and nothing else the App holds. + # + # `repositories:` narrows the repo-level half to the caller. It does not + # narrow the board: `organization_projects` is an ORG permission, measured + # unaffected by repo scoping in backend#2181's run 32255581084. In a + # reusable this expands to the CALLING repository, which is the one whose + # issue is being resolved. + # + # Deliberately NOT a twelfth unscoped mint (backend#2157 is open about the + # eleven this repo already carries) -- `mint-scope.py` would refuse it, and + # this workflow is not on its EXEMPT list. + repositories: ${{ github.event.repository.name }} + permission-issues: read + permission-organization-projects: write + + - name: Promote the card out of Backlog + if: steps.candidate.outputs.verdict == 'yes' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + ORG: ${{ inputs.org }} + PROJECT_NUMBER: ${{ inputs.project-number }} + ISSUE_NUMBER: ${{ github.event.issue.number }} + REPO_FULL: ${{ github.repository }} + run: | + set -euo pipefail + REPO_NAME="${REPO_FULL#*/}" + + # THE RULE, WRITTEN ONCE. Every use below reads these two variables, so + # there is no second spelling of either column to drift. + FROM_STATUS="Backlog" + TO_STATUS="Ready" + + # ONE QUERY for the board AND the card, deliberately. Two queries can + # disagree -- a column renamed between them makes the card's own Status a + # name the options list does not contain, and the monotonicity proof below + # is then being made against a board that no longer exists. `field(name:)` + # also avoids `fields(first: 50)`, whose truncation would silently hide the + # Status field on a wider project. + # shellcheck disable=SC2016 # the $names here are GraphQL variables, not shell - keep literal + QUERY=' + query($org: String!, $repo: String!, $num: Int!, $pnum: Int!) { + organization(login: $org) { + projectV2(number: $pnum) { + id + field(name: "Status") { + ... on ProjectV2SingleSelectField { id options { id name } } + } + } + } + repository(owner: $org, name: $repo) { + issue(number: $num) { + projectItems(first: 20) { + nodes { + id + project { number } + fieldValueByName(name: "Status") { + ... on ProjectV2ItemFieldSingleSelectValue { name } + } + } + } + } + } + }' + + # Retry briefly: `add-to-kanban.yml` and this workflow both fire on + # `issues: opened`, so the card may not exist yet. Budget deliberately the + # SAME 5 x 5s as set-pr-status.yml -- widening it here would hide how often + # the race is lost, and that measurement is the point of failing closed. + RESP="" + ITEM_ID="" + for i in 1 2 3 4 5; do + RESP=$(gh api graphql -f query="$QUERY" -F org="$ORG" -F repo="$REPO_NAME" \ + -F num="$ISSUE_NUMBER" -F pnum="$PROJECT_NUMBER") || RESP="" + if [ -n "$RESP" ]; then + ITEM_ID=$(printf '%s' "$RESP" | jq -r --argjson n "$PROJECT_NUMBER" ' + [.data.repository.issue.projectItems.nodes[]? + | select(.project.number == $n)] | (first // {}) | .id // ""') + fi + if [ -n "$ITEM_ID" ]; then break; fi + echo "issue #$ISSUE_NUMBER not on project $PROJECT_NUMBER yet, waiting ($i/5)…" + sleep 5 + done + + # FAIL CLOSED, for the reason backend#2037 made set-pr-status.yml fail + # closed: a `::notice::` + exit 0 here leaves the card in Backlog behind a + # GREEN check, which is indistinguishable from the bug this workflow was + # written to remove. + if [ -z "$ITEM_ID" ]; then + echo "::error::issue $REPO_FULL#$ISSUE_NUMBER is not on project $PROJECT_NUMBER" \ + "after 5 retries (or the board could not be read). add-to-kanban should have" \ + "added it. Re-run this job once the card exists." + exit 1 + fi + + PROJECT_ID=$(printf '%s' "$RESP" | jq -r '.data.organization.projectV2.id // ""') + STATUS_FIELD=$(printf '%s' "$RESP" | jq -r '.data.organization.projectV2.field.id // ""') + CURRENT_COL=$(printf '%s' "$RESP" | jq -r --argjson n "$PROJECT_NUMBER" ' + [.data.repository.issue.projectItems.nodes[]? + | select(.project.number == $n)] | (first // {}) + | .fieldValueByName.name // ""') + if [ -z "$PROJECT_ID" ] || [ -z "$STATUS_FIELD" ]; then + echo "::error::project $PROJECT_NUMBER has no readable 'Status' single-select field." \ + "Refusing to guess a column id." + exit 1 + fi + + # Position on the board, asked OF the board. A hand-written rank table is a + # second copy of the column order that agrees with itself while disagreeing + # with reality (backend#1729 rule 1); the option list already comes back in + # pipeline order, which is what `kanban-closure-router.yml` relies on too. + col_index() { + printf '%s' "$RESP" | jq -r --arg s "$1" \ + '[.data.organization.projectV2.field.options[].name] | index($s) // -1' + } + + # selftest:gate-start + # MONOTONIC BY CONSTRUCTION, not by a list of columns to avoid. + # + # promote the card is in FROM_STATUS (or on the board with no Status at + # all) -- the only two states from which moving to TO_STATUS is + # forward. + # hold anywhere else. A bug labelled while the card is already at + # `In progress`, `On dev` or `Prod` must NOT be dragged back to + # `Ready`; the label can arrive at any point in a card's life and + # usually arrives after triage has already moved it. This is the + # common case and it is GREEN and logged. + # noboard the two anchors are missing, or TO_STATUS does not sit strictly + # AFTER FROM_STATUS on the live board. That second half is the + # monotonicity assertion itself: if someone drags the Status + # options so `Ready` precedes `Backlog`, then this workflow's + # "promotion" is a demotion, and it must refuse rather than + # perform it. Existence was checked and ORDER was not in the + # sibling classifier until backend#1994; that hole is not + # reproduced here. + # unknown the card sits in a column the board did not report. Unreachable + # from a single atomic read, which is why the read above is one + # query -- kept so the `case` has no fall-through, and driven by + # the selftest against a synthetic board. + promotion_verdict() { + _f=$(col_index "$FROM_STATUS"); _t=$(col_index "$TO_STATUS") + if [ "$_f" -lt 0 ] || [ "$_t" -lt 0 ] || [ "$_f" -ge "$_t" ]; then + echo noboard; return + fi + case "${1:-}" in + ""|"No status") echo promote; return ;; + esac + _c=$(col_index "$1") + if [ "$_c" -lt 0 ]; then echo unknown; return; fi + if [ "$_c" -eq "$_f" ]; then echo promote; else echo hold; fi + } + + _v=$(promotion_verdict "${CURRENT_COL:-}") + case "$_v" in + noboard) + echo "::error::the board's '$FROM_STATUS' and '$TO_STATUS' columns are missing," \ + "or '$TO_STATUS' does not sit after '$FROM_STATUS' — a move between them" \ + "cannot be shown to be forward, so it will not be made." + exit 1 ;; + unknown) + # LOUD, unlike the sibling classifier's quiet `unknown` in + # kanban-closure-router.yml, and the difference is deliberate. There, + # declining is the conservative end state and the card is legitimately + # parked. Here it means the monotonicity proof could not be made from a + # read that should have been atomic -- "cannot tell", which is a finding. + echo "::error::card for #$ISSUE_NUMBER sits in '${CURRENT_COL}', which this" \ + "board does not list as a Status option. Refusing to move it." + exit 1 ;; + hold) + echo "::notice::#$ISSUE_NUMBER is at '${CURRENT_COL:-}', not '$FROM_STATUS'" \ + "— leaving it there. Automation never moves a card backward." + exit 0 ;; + esac + # selftest:gate-end + + TO_OPT=$(printf '%s' "$RESP" | jq -r --arg s "$TO_STATUS" \ + '[.data.organization.projectV2.field.options[] | select(.name == $s) | .id] + | (first // "")') + if [ -z "$TO_OPT" ]; then + echo "::error::'$TO_STATUS' resolved no option id on project $PROJECT_NUMBER." + exit 1 + fi + + # `-f` for the option id, never `-F`: ProjectV2 option ids can be + # all-numeric and `-F` coerces an all-digit value to an integer, which the + # `$o: String!` variable then rejects. + # shellcheck disable=SC2016 # the $names here are GraphQL variables, not shell - keep literal + gh api graphql -f query=' + mutation($p: ID!, $i: ID!, $f: ID!, $o: String!) { + updateProjectV2ItemFieldValue(input: { + projectId: $p, itemId: $i, fieldId: $f, + value: {singleSelectOptionId: $o} + }) { projectV2Item { id } } + }' -F p="$PROJECT_ID" -F i="$ITEM_ID" -F f="$STATUS_FIELD" -f o="$TO_OPT" > /dev/null + + echo "→ $REPO_FULL#$ISSUE_NUMBER carries the bug label: '${CURRENT_COL:-}' → '$TO_STATUS'" diff --git a/.github/workflows/kanban-columns.yml b/.github/workflows/kanban-columns.yml index adb27c1..d92813b 100644 --- a/.github/workflows/kanban-columns.yml +++ b/.github/workflows/kanban-columns.yml @@ -29,6 +29,7 @@ on: # ever checked by the daily cron. - ".github/workflows/kanban-archive.yml" - ".github/workflows/wip-limit-check.yml" + - ".github/workflows/bug-to-ready.yml" - ".github/workflows/kanban-columns.yml" # The mapping is an INPUT to the check now (backend#2243), so a PR that only # touches it must run this (Bugbot, .github#295) -- otherwise a new Status can diff --git a/Makefile b/Makefile index a8ec539..ae5dff5 100644 --- a/Makefile +++ b/Makefile @@ -302,7 +302,7 @@ mutations-dry: $(addsuffix -dry,$(MUTATION_TARGETS)) SELFTEST_TARGETS := selftest-caller-drift selftest-blocked-marker selftest-standards-sync \ selftest-stale-backlog \ selftest-version-bump-gate selftest-bricked-prs selftest-kanban-columns \ - selftest-kanban-deploy-state selftest-git-reap \ + selftest-kanban-deploy-state selftest-bug-to-ready selftest-git-reap \ selftest-mint-scope selftest-house-rules \ selftest-pipefail-early-close @@ -429,6 +429,16 @@ selftest-kanban-columns: selftest-kanban-deploy-state: guard-pyyaml $(PYTHON) scripts/tests/kanban-deploy-state-selftest.py +# guard-pyyaml: it parses bug-to-ready.yml to pull the two decision regions out +# of the workflow rather than holding a copy of them (backend#2348). NO CI +# workflow of its own, deliberately — `selftests.yml` is already a REQUIRED +# context that runs `make selftests`, so a new path-filtered workflow would add +# an unrequired job and a filter to keep in step, which is the shape +# selftests.yml's own header argues against. +.PHONY: selftest-bug-to-ready +selftest-bug-to-ready: guard-pyyaml + $(PYTHON) scripts/tests/bug-to-ready-selftest.py + # Builds a throwaway repo per case and stubs `gh` on PATH, so it needs neither a # token nor a network — but it DOES need a committer identity, which a bare CI # runner lacks. git-reap-selftest.yml configures one; a developer machine diff --git a/repo-inventory.yml b/repo-inventory.yml index 9fb8c85..90127b0 100644 --- a/repo-inventory.yml +++ b/repo-inventory.yml @@ -111,6 +111,7 @@ reusables: - blocked-gate.yml - version-bump-gate.yml - stale-backlog.yml + - bug-to-ready.yml # Copies, not callers: these are duplicated into each repo, so the guard compares # content (git blob sha against the checked-out source) instead of looking for a @@ -358,6 +359,20 @@ shared_reasons: the moment it is on - client#490 ("HOLD until v0.8.0 image") and client-runtime#192 ("DO NOT MERGE") - which is the gate working as intended (backend#1752, data-ingestors#468). + bug_to_ready_rollout_pending: &bug_to_ready_rollout_pending >- + STAGED, NOT PARKED (backend#2348). bug-to-ready.yml lands with ZERO callers, deliberately + and for exactly one PR: the reusable plus its selftest so the PROMOTION DECISION can be + reviewed on its own -- that is the whole risk surface, because the way this workflow fails + is not "it does nothing", it is dragging a card that already reached In progress / On dev / + Prod backwards into Ready. The callers are four lines of YAML each and follow in the rollout + PR, which flips every one of these rows to `required`. + The two-step is forced by this file's own header: .github's caller state is read from its + audit branch over the API, so adding a caller and flipping its entry in one PR fails on a + caller that is not on the audit branch yet. .github IS IN THE WAVE LIKE EVERY OTHER REPO -- + the same reasoning stale_backlog_migration_in_flight spells out, for the same reason. + If this anchor is still here once the rollout has merged, it has become the same finding as + wip_limit_check_has_no_callers: a reusable that shipped, was never wired up, and had a + written excuse for it. Treat it as a defect then, not a justification. no_staging_branch_no_hop_to_gate: &no_staging_branch_no_hop_to_gate >- fr-gate gates the staging -> prod hop. This repo has no `staging` branch, so there is no hop @@ -562,6 +577,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -595,6 +612,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -627,6 +646,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -680,6 +701,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -711,6 +734,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -742,6 +767,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -775,6 +802,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -806,6 +835,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -837,6 +868,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -905,6 +938,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -952,6 +987,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1037,6 +1074,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1076,6 +1115,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1144,6 +1185,8 @@ repos: exempt: *blocked_gate_rollout_pending stale-backlog.yml: exempt: *stale_backlog_exemption_needs_redeciding + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: divergent: >- @@ -1227,6 +1270,8 @@ repos: exempt: *blocked_gate_rollout_pending stale-backlog.yml: exempt: *stale_backlog_not_a_backlog + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1264,6 +1309,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1297,6 +1344,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1328,6 +1377,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: @@ -1366,6 +1417,8 @@ repos: exempt: *wip_limit_check_has_no_callers blocked-gate.yml: exempt: *blocked_gate_rollout_pending + bug-to-ready.yml: + exempt: *bug_to_ready_rollout_pending copies: add-to-kanban.yml: required quality_files: diff --git a/scripts/kanban-columns-check.py b/scripts/kanban-columns-check.py index 858aea5..15797bd 100755 --- a/scripts/kanban-columns-check.py +++ b/scripts/kanban-columns-check.py @@ -89,6 +89,12 @@ # because the paths-filter assertion and the idiom cross-check both key on it. "kanban-archive.yml", "wip-limit-check.yml", + # Writes `Ready` and reads `Backlog` to decide whether that write is forward + # (backend#2348). Both names are load-bearing in opposite directions: rename + # `Ready` and the write resolves no option id, rename `Backlog` and the + # workflow refuses every promotion -- so a board rename turns it into either + # a red run or a permanent no-op, which is exactly what this check is for. + "bug-to-ready.yml", ) diff --git a/scripts/tests/bug-to-ready-selftest.py b/scripts/tests/bug-to-ready-selftest.py new file mode 100644 index 0000000..96d34ae --- /dev/null +++ b/scripts/tests/bug-to-ready-selftest.py @@ -0,0 +1,331 @@ +#!/usr/bin/env python3 +"""The bug-label promotion is read OUT of `bug-to-ready.yml` and exercised. + +WHY THIS EXISTS (backend#2348) + +The org rule "bugs get `work-type:bug` and go straight into `Ready`" had a 100% +miss rate because nothing implemented it. `bug-to-ready.yml` implements it, and +the failure mode of a workflow like that is not "it does not work" -- it is that +it works in the one case someone tried and quietly does the WRONG thing in the +others. The wrong thing here is destructive: dragging a card that has reached +`In progress`, `On dev` or `Prod` backwards into `Ready`. + +So the cases that matter most in this file are the ones where it must do +NOTHING. + +WHY IT IS EXTRACTED RATHER THAN COPIED + +A copy of the decision here would let the workflow drift while this file stays +green -- the defect class backend#1729 catalogued, and the one .github#114/#115 +hit twice in a day by re-implementing the rule inside the check. Both regions +under test are pulled out of the YAML by their `# selftest:` markers and run +verbatim by bash. If someone renames or reshapes them, this test stops finding +them and fails loudly rather than testing a stale duplicate. + +WHAT IS EXTRACTED + + # selftest:candidate-* Is this event a bug-labelled ISSUE at all? Driven + through its env inputs -- event name, the + is-a-pull-request flag, the raw labels JSON. + # selftest:gate-* Given the card's current column, is moving it to + `Ready` forward? Driven through a synthetic board via + a stubbed `col_index`, which is the same seam the + workflow's real one presents. + +THE BOARD IS NOT RESTATED HERE. `kanban-deploy-state-selftest.py` already holds +the live board's option order, and a second hand-written copy of it in the same +directory is the drift shape this repo keeps finding. It is parsed out of that +file with `ast`, so a rename there fails this loudly instead of leaving two +lists to disagree. + +Exit 0 when every case behaves as specified. +""" +from __future__ import annotations + +import ast +import os +import re +import shlex +import subprocess +import sys + +import yaml + +HERE = os.path.dirname(os.path.abspath(__file__)) +WORKFLOWS = os.path.join(HERE, os.pardir, os.pardir, ".github", "workflows") +WF = os.path.join(WORKFLOWS, "bug-to-ready.yml") +SIBLING = os.path.join(HERE, "kanban-deploy-state-selftest.py") + +RESULTS: "list[tuple[bool, str, str]]" = [] + + +def record(ok: bool, name: str, detail: str) -> None: + RESULTS.append((ok, name, detail)) + print(f"{'PASS' if ok else 'FAIL'} {name}\n {detail}") + + +def dedent(block: str) -> str: + indent = min((len(ln) - len(ln.lstrip()) for ln in block.splitlines() + if ln.strip()), default=0) + return "\n".join(ln[indent:] if ln[:indent].isspace() else ln + for ln in block.splitlines()) + + +def region(marker: str) -> str: + """The workflow's own shell for `# selftest:-start|end`, verbatim.""" + doc = yaml.safe_load(open(WF)) + bodies = [s["run"] for j in doc["jobs"].values() + for s in j.get("steps", []) if "run" in s] + pattern = (rf"^[ \t]*# selftest:{marker}-start\b.*?" + rf"^[ \t]*# selftest:{marker}-end\b[^\n]*$") + for body in bodies: + m = re.search(pattern, body, re.S | re.M) + if m: + return dedent(m.group(0)) + sys.exit(f"could not find the # selftest:{marker}-* region in " + f"{os.path.basename(WF)} — was it renamed or its markers dropped? " + "This test refuses to fall back to a copy of the logic.") + + +def board_from_sibling() -> "list[str]": + """The live board's option ORDER, parsed out of the file that already holds it. + + Derived rather than restated (backend#1729 rule 1). A second literal list here + would agree with itself forever while the board and its sibling moved on. + """ + tree = ast.parse(open(SIBLING).read()) + for node in tree.body: + if isinstance(node, ast.Assign) and any( + isinstance(t, ast.Name) and t.id == "BOARD" for t in node.targets + ): + value = ast.literal_eval(node.value) + if isinstance(value, list) and all(isinstance(v, str) for v in value): + return value + sys.exit(f"no module-level `BOARD = [...]` literal in {SIBLING}. It moved or " + "was renamed; this test will not fall back to its own copy of the " + "board order.") + + +BOARD = board_from_sibling() +for anchor in ("Backlog", "Ready"): + if anchor not in BOARD: + sys.exit(f"the parsed board has no {anchor!r} column, so nothing below " + "would be testing the rule this workflow implements.") + + +def sh(script: str, env: "dict[str, str] | None" = None) -> "tuple[int, str]": + full = dict(os.environ) + full.update(env or {}) + out = subprocess.run(["bash", "-c", script], capture_output=True, text=True, + env=full) + return out.returncode, (out.stdout + out.stderr).strip() + + +# =========================================================================== +# 1. THE CANDIDATE GATE — which events are even eligible. +# =========================================================================== +CANDIDATE = region("candidate") + + +def candidate(event: str, is_pr: str, labels_json: str, + bug_label: str = "work-type:bug") -> "tuple[int, str]": + rc, out = sh("set -euo pipefail\n" + CANDIDATE + '\necho "VERDICT=$VERDICT"', + {"EVENT_NAME": event, "IS_PULL_REQUEST": is_pr, + "LABELS_JSON": labels_json, "BUG_LABEL": bug_label}) + verdict = "" + for line in out.splitlines(): + if line.startswith("VERDICT="): + verdict = line[len("VERDICT="):] + return rc, (verdict if rc == 0 else f"rc={rc}|{out}") + + +def labels(*names: str) -> str: + import json + return json.dumps([{"name": n} for n in names]) + + +# 1a. The case the ticket is about: a bug-labelled issue is a candidate. +rc, v = candidate("issues", "false", labels("work-type:bug")) +record(rc == 0 and v == "yes", "a bug-labelled issue is a candidate", f"-> {v}") + +rc, v = candidate("issues", "false", + labels("priority", "work-type:bug", "from:customer")) +record(rc == 0 and v == "yes", + "the label is found among several", f"-> {v}") + +# 1b. NOT A CANDIDATE, and each of these is a way the workflow could have done +# damage or noise instead. +for why, args in { + "an unlabelled issue": ("issues", "false", labels()), + "an issue labelled something else": ("issues", "false", + labels("work-type:feature")), +}.items(): + rc, v = candidate(*args) + record(rc == 0 and v == "no:label-absent", f"{why} is not a candidate", + f"-> {v}") + +# 1c. NEAR MISSES. The match must be the WHOLE label name. A substring or +# case-insensitive match would promote cards nobody labelled as a defect, +# and `work-type:bug` is a prefix of nothing today — which is exactly why +# the inputs are written down here independently of the matcher rather than +# generated from it (backend#1729 rule 9's corollary). +for near in ("work-type:bugfix", "Work-Type:Bug", "WORK-TYPE:BUG", "bug", + "type:bug", " work-type:bug", "work-type:bug "): + rc, v = candidate("issues", "false", labels(near)) + record(rc == 0 and v == "no:label-absent", + f"{near!r} is not {'work-type:bug'!r}", f"-> {v}") + +# 1d. PULL REQUESTS, NEVER. `issues:` events do not fire for PRs, so the caller +# already excludes them — but a reusable can be called from any trigger, and +# `issue_comment` fires for both. Per the board's model a PR lives in the +# flow columns, so promoting one to `Ready` would be a demotion of real work. +rc, v = candidate("issues", "true", labels("work-type:bug")) +record(rc == 0 and v == "no:pull-request", + "a PR carrying the bug label is refused, label or not", f"-> {v}") + +rc, v = candidate("issue_comment", "false", labels("work-type:bug")) +record(rc == 0 and v == "no:not-an-issues-event", + "a non-`issues` event is refused before anything else", f"-> {v}") + +# 1e. CANNOT TELL IS A FINDING (backend#1729 rule 3). Each of these used to be +# the easy fail-open: unparseable labels reading as "not a bug", and the +# card staying in Backlog behind a green check — indistinguishable from the +# bug this workflow removes. +for why, payload in { + "labels that are not JSON": "not json at all", + "an empty labels payload": "", + "labels that are JSON but not a list": '{"name": "work-type:bug"}', +}.items(): + rc, out = candidate("issues", "false", payload) + record(rc == 1 and "::error::" in out, + f"{why} fails the run rather than reading as 'no'", + out.splitlines()[0] if out else "") + +rc, out = candidate("issues", "false", labels("work-type:bug"), bug_label="") +record(rc == 1 and "::error::" in out, + "an empty `bug-label` input fails rather than matching everything", + out.splitlines()[0] if out else "") + +# =========================================================================== +# 2. THE MONOTONICITY GATE — the half that must refuse. +# =========================================================================== +GATE = region("gate") + +# The marker is only reached when the gate falls through to the write, so its +# presence IS the promote verdict; `hold` exits 0 before it and the two refusals +# exit 1. Nothing here re-implements the decision. +WOULD_WRITE = "WOULD-WRITE" + + +def gate(current: str, board: "list[str]" = None) -> "tuple[int, str]": + names = BOARD if board is None else board + script = f""" +set -euo pipefail +ISSUE_NUMBER=1 +FROM_STATUS="Backlog" +TO_STATUS="Ready" +BOARD_NAMES={shlex.quote(chr(10).join(names))} +# The same contract the workflow's own col_index presents: index on the board, +# or -1. Stubbed so the board can be reshaped; the DECISION below is the +# workflow's own text. +col_index() {{ + _i=0 + while IFS= read -r _n; do + if [ "$_n" = "$1" ]; then echo "$_i"; return; fi + _i=$((_i + 1)) + done < str: + if rc != 0: + if "does not sit after" in out: + return "noboard" + if "does not list as a Status option" in out: + return "unknown" + return f"error({rc}):{out[:120]}" + return "promote" if WOULD_WRITE in out else "hold" + + +# 2a. THE ONE COLUMN THAT PROMOTES, plus the two no-Status spellings. A card +# that reached the board but carries no Status is not "past Ready" — leaving +# it unplaced would keep it invisible, which is the whole complaint. +for col in ("Backlog", "", "No status"): + rc, out = gate(col) + record(verdict_of(rc, out) == "promote", + f"a card at {col or ''!r} is promoted", + f"-> {verdict_of(rc, out)}") + +# 2b. THE WHOLE VOCABULARY, DERIVED FROM THE BOARD (backend#1729 rule 6). +# Mutation coverage cannot see a vocabulary gap, so every column the board +# declares is driven through the gate and exactly ONE of them may promote. +# `Prod`, `Done` and `Cancelled` are the expensive ones: a card that shipped +# or was cancelled must never be dragged back into the pull queue by someone +# labelling it after the fact. +promoted = [c for c in BOARD if verdict_of(*gate(c)) == "promote"] +record(promoted == ["Backlog"], + "of all %d board columns, exactly 'Backlog' promotes" % len(BOARD), + f"promoting: {promoted or 'none'}; every other column holds") + +for col in BOARD: + if col == "Backlog": + continue + rc, out = gate(col) + v = verdict_of(rc, out) + record(v == "hold" and rc == 0, + f"a card already at {col!r} is left alone", + f"-> {v}; automation must never move a card backward") + +# 2c. A COLUMN THE BOARD DOES NOT REPORT. Unreachable from the workflow's single +# atomic read, and kept so the `case` has no fall-through. Driven here +# because an arm nothing exercises is an arm nobody knows the sign of. +rc, out = gate("Some Column Nobody Declared") +record(verdict_of(rc, out) == "unknown" and rc == 1, + "a column the board does not list refuses LOUDLY", + "the monotonicity proof could not be made, so 'cannot tell' is a finding") + +# 2d. A BOARD MISSING AN ANCHOR cannot place anything. +for gone in ("Backlog", "Ready"): + trimmed = [c for c in BOARD if c != gone] + assert gone not in trimmed and len(trimmed) == len(BOARD) - 1, trimmed + rc, out = gate("Backlog" if gone == "Ready" else "Ready", trimmed) + record(verdict_of(rc, out) == "noboard" and rc == 1, + f"a board with no {gone!r} column refuses rather than guessing", + f"-> {verdict_of(rc, out)}") + +# 2e. BOTH ANCHORS PRESENT, ORDER INVERTED. This is the monotonicity assertion +# itself, and it is the hole backend#1994 found in the sibling classifier: +# existence was checked and ORDER was not. Drag the Status options so +# `Ready` sorts before `Backlog` and this workflow's "promotion" becomes a +# demotion — it must refuse to make it. +b_i, r_i = BOARD.index("Backlog"), BOARD.index("Ready") +inverted = list(BOARD) +inverted[b_i], inverted[r_i] = inverted[r_i], inverted[b_i] +# An inert input and a working guard produce the same green line, so assert the +# board really is inverted and otherwise UNCHANGED. +assert inverted.index("Ready") < inverted.index("Backlog"), inverted +assert sorted(inverted) == sorted(BOARD), inverted +rc, out = gate("Ready", inverted) +record(verdict_of(rc, out) == "noboard" and rc == 1, + "a board whose Backlog/Ready order is INVERTED refuses", + "moving 'forward' on that board is a demotion — existence is not enough") +# And the card sitting in Backlog on that same inverted board must ALSO refuse: +# the anchor check has to precede the current-column check, or the one case that +# would actually perform the demotion sails past it. +rc, out = gate("Backlog", inverted) +record(verdict_of(rc, out) == "noboard" and rc == 1, + "the inverted board refuses even for a card at 'Backlog'", + "the anchor check runs BEFORE the current-column check, or the single " + "demoting case is the one that escapes") + +failed = [r for r in RESULTS if not r[0]] +print(f"\n{len(RESULTS) - len(failed)} passed, {len(failed)} failed") +sys.exit(1 if failed else 0)