diff --git a/.github/workflows/customer-priority-bump.yml b/.github/workflows/customer-priority-bump.yml index f02b4c7..5768cbf 100644 --- a/.github/workflows/customer-priority-bump.yml +++ b/.github/workflows/customer-priority-bump.yml @@ -1,18 +1,60 @@ -name: Bump priority on customer-flagged issue +name: Label-driven issue triage # Reusable workflow, called by a thin `customer-priority-bump.yml` caller in # each wired repo on issues.types=labeled (callers pass `secrets: inherit` and -# no inputs). When the trigger label (default 'from:customer') is added to an -# issue, this adds the binary 'priority' label to that same issue (D5). -# It writes no board field — the Priority single-select was removed from the -# board under D5. +# no inputs). TWO label rules live here, both keyed on the label that was just +# added: +# +# from:customer -> add the binary `priority` label to the issue (D5). +# Writes no board field -- the Priority single-select was +# removed from the board under D5. +# work-type:bug -> move the issue's kanban card from `Backlog` to `Ready` +# (backend#2348). Defects skip refinement. +# +# WHY BOTH LIVE IN ONE FILE, AND WHY THE FILENAME NO LONGER MATCHES +# ---------------------------------------------------------------- +# The bug rule needs exactly one thing: to run on `issues: labeled` in every +# repo. This reusable is the ONLY place in the org that already does, and 16 of +# the 19 repos already call it. Adding a second reusable instead would need a +# `repo-inventory.yml` row for all 19 repos plus a caller rollout -- and +# `repo-inventory.yml` is guarded by `conformance-gate.yml`, so that change +# cannot merge until an org audit passes on its exact head sha, which every other +# merge invalidates. A rule that has been unimplemented since it was written does +# not need to wait behind that; it needs to run. +# +# The cost is honest and stated: the file name and the 16 per-repo caller names +# still say "customer priority bump", so a bug-label run shows up in each repo's +# Actions tab under that name. The workflow's own `name:` above is the half that +# could be fixed without touching 16 repos, so it was. Renaming the file is a +# caller rollout (BUGBOT.md property 1: land the callee first) and is left for +# whoever does the `.github`/`release-train`/`rfcs` wiring below. +# +# NOT WIRED EVERYWHERE, and that is a real gap rather than a rounding error: +# `.github`, `release-train` and `rfcs` are `exempt` for this reusable in +# `repo-inventory.yml` (`customer_priority_bump_caller_missing`), so a bug filed +# in one of those three still lands in `Backlog` and still needs a hand. The +# other 16 -- including `backend` and `frontend-app`, where all seven of +# backend#2348's measured misses were filed -- are covered. on: workflow_call: inputs: trigger-label: + description: "The label that means 'a customer asked for this'" type: string default: "from:customer" + bug-label: + description: "The label that means 'this is a defect' (skips refinement)" + type: string + default: "work-type:bug" + project-number: + description: "GitHub Projects v2 number (default: 2 = engineer kanban)" + type: number + default: 2 + org: + description: "GitHub org owning the project" + type: string + default: tracebloc jobs: bump: @@ -45,3 +87,399 @@ jobs: # Priority single-select field has been removed from the board. gh issue edit "$ISSUE_NUMBER" --repo "$REPO_FULL" --add-label priority echo "-> Issue #$ISSUE_NUMBER labelled 'priority'" + + # --------------------------------------------------------------------------- + # A bug-labelled issue lands in `Ready`, not `Backlog` (backend#2348) + # --------------------------------------------------------------------------- + # THE RULE, quoted from `org-standards.md`: "label them `work-type:bug` (the + # Bug template does it) and the board moves the card straight into `Ready`". + # + # Nothing implemented it. `add-to-kanban.yml` adds every new issue at + # `Backlog` and no workflow read the label afterwards, so the rule was carried + # by whoever remembered. 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 all seven needed a hand-run + # mutation. A 100% miss rate is the tell that nothing does it at all -- a rule + # people mostly follow produces a mixed record. + # + # `Ready` is the queue engineers pull from; `Backlog` is the refinement queue + # nobody pulls from. That split is the whole point, so a defect filed correctly + # and labelled correctly was invisible to the people meant to pick it up. + bug-to-ready: + # A COST GATE, not the decision. It exists so that an unrelated label -- and + # every repo in the fleet fires this workflow on every `labeled` event -- does + # not mint an App token. The decision is `label_gate` in the step below, where + # it can be extracted, run and mutated by + # `scripts/tests/bug-to-ready-selftest.py`. `==` here is exact string + # equality, so this can only ever be STRICTER than that gate: it may skip work + # the gate would decline, never admit work the gate refuses. + if: github.event_name == 'issues' && github.event.label.name == inputs.bug-label + runs-on: ubuntu-latest + timeout-minutes: 10 + # Nothing here uses the caller's GITHUB_TOKEN: every call is made with the + # App installation token minted below. + permissions: {} + steps: + # LEAST PRIVILEGE, DERIVED FROM WHAT THIS RUNS (backend#2157), unlike the + # `bump` job above which still carries the App's full installation grant + # (`mint-scope.py`'s EXEMPT row for this file is about that job, not this + # one). This step reads one issue's project items and writes one + # single-select field on an org project -- so `issues: read` plus + # `organization-projects: write` is the whole requirement. + # + # `repositories:` is deliberately NOT narrowed here. The board write needs + # the ORG-level grant that `owner:` yields, and the interaction between repo + # narrowing and an org ProjectV2 write is measured for a READ + # (kanban-columns.yml, backend#2181) and not for a write. An unmeasured + # narrowing on a workflow that fires for every bug in the fleet fails red on + # every defect filed, and this job's whole point is that a defect should not + # need a human to notice it. + - name: Mint an installation token + id: app-token + 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 }} + permission-issues: read + permission-organization-projects: write + + - name: Promote the card to Ready, but only from Backlog + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + ORG: ${{ inputs.org }} + PROJECT_NUMBER: ${{ inputs.project-number }} + BUG_LABEL: ${{ inputs.bug-label }} + # THE TWO ANCHORS, QUOTED SO THE BOARD CHECK CAN SEE THEM. + # `scripts/kanban-columns-check.py` collects every board column name a + # WRITERS workflow quotes on a code line and asserts it exists on the + # live board -- which is what stops a rename in the Projects UI turning + # this job into a silent no-op. Unquoted YAML values are invisible to it. + SOURCE_COLUMN: "Backlog" + TARGET_COLUMN: "Ready" + EVENT_NAME: ${{ github.event_name }} + LABEL_ADDED: ${{ github.event.label.name }} + # A PR is not an issue. `issues` events never fire for pull requests, so + # this is belt and braces for a caller wired to `pull_request: labeled` + # -- both payload shapes are checked because they carry the PR in + # different places. + HAS_PR_PAYLOAD: ${{ github.event.pull_request != null || github.event.issue.pull_request != null }} + NUMBER: ${{ github.event.issue.number }} + REPO_FULL: ${{ github.repository }} + run: | + set -euo pipefail + REPO_NAME="${REPO_FULL#*/}" + + # ---- 1. is this an event this job may act on? -------------------- + # selftest:label-gate-start + # THE AUTHORITATIVE EVENT/LABEL GATE. The job-level `if:` above is a + # cost gate; this is the decision, written where a test can run it. + # + # EVERY REFUSAL NAMES ITSELF. A gate with five refusal paths and one + # bare failure cannot tell a test which path it took, so a case goes on + # passing while exercising a different refusal than its name claims + # (CLAUDE.md rule 10). + label_gate() { # $1=event $2=label added $3=label we act on $4=PR payload? + if [ "${1:-}" != "issues" ]; then echo "refuse:not-an-issues-event"; return; fi + if [ "${4:-}" = "true" ]; then echo "refuse:pull-request-payload"; return; fi + if [ -z "${2:-}" ]; then echo "refuse:unreadable-label"; return; fi + if [ -z "${3:-}" ]; then echo "refuse:no-configured-label"; return; fi + if [ "$2" != "$3" ]; then echo "refuse:other-label"; return; fi + echo proceed + } + GATE=$(label_gate "${EVENT_NAME:-}" "${LABEL_ADDED:-}" "${BUG_LABEL:-}" "${HAS_PR_PAYLOAD:-}") + case "$GATE" in + proceed) + echo "'${LABEL_ADDED}' added to issue #${NUMBER} - evaluating its card" ;; + refuse:other-label) + # THE ONLY GREEN REFUSAL, and the only one that is a normal event: + # this workflow runs on every `labeled` event in every wired repo, + # so most runs land here. + echo "::notice::'${LABEL_ADDED}' is not '${BUG_LABEL}' - nothing to do" + exit 0 ;; + *) + # Every other refusal is a payload this decision was never written + # for. Unreachable through the `if:` above, which is exactly why it + # is LOUD: if it ever fires, a caller has been wired to an event + # this job cannot judge, and a quiet exit 0 would hide that for as + # long as nobody reads the run log. + echo "::error::${GATE}: this job promotes a bug-labelled ISSUE and cannot judge this payload" >&2 + exit 1 ;; + esac + # selftest:label-gate-end + + # Every GraphQL call in this job -- both reads AND the write -- goes through + # here. `gh api graphql` exits 0 on an HTTP 200 that carries a GraphQL + # `errors[]` payload, so an exit code alone cannot tell a completed + # operation from a refused one. ONE function rather than a check per call + # site: the two reads each carried their own inline copy and the write + # carried none, which is exactly how the write came to be fail-open + # (Bugbot, .github#313). Rule 1 (derive, never restate) and rule 9 -- the + # selftest extracts THIS function by name, so the assertion and the + # mutation drive the same code the job runs. + reject_graphql_errors() { # $1=raw payload $2=the message to fail with + # UNPARSEABLE IS ITS OWN ARM, and it has to come first. `jq -e + # 'has("errors")'` exits non-zero BOTH when the key is absent and when + # the input is not JSON at all, so a single check would read a truncated + # or empty body as "no errors" -- fail-open on precisely the input that + # means "cannot tell". + if ! jq -e . >/dev/null 2>&1 <<< "$1"; then + echo "::error::$2 (the response was not readable JSON, so it cannot be" \ + "shown to be error-free)" >&2 + return 1 + fi + if jq -e 'has("errors")' <<< "$1" >/dev/null 2>&1; then + echo "::error::$2" >&2 + return 1 + fi + return 0 + } + + + # ---- 2. the board, read ONCE ------------------------------------ + # One query for the project id, the Status field id, the target option + # id AND the option ORDER. The order is what the monotonic gate is + # derived from, so reading it in the same response as the ids means the + # decision and the write cannot be made against two different boards. + # + # FAIL CLOSED on the read (backend#1729 rule 3): an unreadable board is + # not evidence that the card may move. + # shellcheck disable=SC2016 # the $names are GraphQL variables, not shell - keep literal + if ! PROJ=$(gh api graphql -f query=' + query($org: String!, $num: Int!) { + organization(login: $org) { + projectV2(number: $num) { + id + fields(first: 50) { + totalCount + nodes { + ... on ProjectV2SingleSelectField { id name options { id name } } + } + } + } + } + }' -F org="$ORG" -F num="$PROJECT_NUMBER"); then + echo "::error::could not read project #${PROJECT_NUMBER} - refusing to guess where this card sits" >&2 + exit 1 + fi + # A GraphQL `errors[]` payload AT EXIT 0 is a partial read, and a partial + # read of the option list is exactly the input that makes a position + # comparison meaningless (the shape `bugbot-gate.py` pins a mutation for). + reject_graphql_errors "$PROJ" \ + "the project read came back with GraphQL errors - a partial board is not a board" || exit 1 + + FIELD_TOTAL=$(jq -r '.data.organization.projectV2.fields.totalCount // -1' <<< "$PROJ") + # A field list longer than the page read means `Status` may be on a page + # nobody looked at, and "absent from the page I read" is not "absent". + if [ "$FIELD_TOTAL" -lt 0 ] || [ "$FIELD_TOTAL" -gt 50 ]; then + echo "::error::project #${PROJECT_NUMBER} reported ${FIELD_TOTAL} fields against a page of 50 -" \ + "the Status field may be unread. Paginate rather than treating a truncated read as complete." >&2 + exit 1 + fi + + PROJECT_ID=$(jq -r '.data.organization.projectV2.id // ""' <<< "$PROJ") + STATUS_FIELD=$(jq -r '.data.organization.projectV2.fields.nodes[]? + | select(.name=="Status") | .id' <<< "$PROJ") + # THE OPTION ID IS DERIVED, NEVER HELD. A stored `Ready` option id would + # keep writing after the board changed under it, and writing the WRONG + # column is strictly worse than writing nothing (backend#2348). + TARGET_OPT=$(jq -r --arg s "$TARGET_COLUMN" '.data.organization.projectV2.fields.nodes[]? + | select(.name=="Status") | .options[] | select(.name==$s) | .id' <<< "$PROJ") + if [ -z "$PROJECT_ID" ] || [ "$PROJECT_ID" = "null" ] \ + || [ -z "$STATUS_FIELD" ] || [ "$STATUS_FIELD" = "null" ] \ + || [ -z "$TARGET_OPT" ] || [ "$TARGET_OPT" = "null" ]; then + echo "::error::could not resolve the Status field or its '${TARGET_COLUMN}' option in" \ + "project #${PROJECT_NUMBER}. NOTHING WAS WRITTEN - this runs before any mutation." >&2 + exit 1 + fi + + # ---- 3. where is the card now? ---------------------------------- + # THE ISSUE'S OWN projectItems, not a scan of the project. Project #2 + # carries ~700 items, so a project-side scan is a pagination bug waiting + # to happen; the issue knows which cards it has. `totalCount` is read so + # a card beyond the page cannot be reported as "not on the board". + # + # The retry is for the `opened`+template case: the Bug template applies + # the label at creation, so this can race `add-to-kanban.yml`. + ITEM_ID=""; CURRENT_COL=""; ARCHIVED="" + for attempt in 1 2 3 4 5; do + # shellcheck disable=SC2016 # the $names are GraphQL variables, not shell - keep literal + if ! IRESP=$(gh api graphql -f query=' + query($org: String!, $repo: String!, $num: Int!) { + repository(owner: $org, name: $repo) { + issue(number: $num) { + projectItems(first: 20) { + totalCount + nodes { + id isArchived project { number } + status: fieldValueByName(name: "Status") { + ... on ProjectV2ItemFieldSingleSelectValue { name } + } + } + } + } + } + }' -F org="$ORG" -F repo="$REPO_NAME" -F num="$NUMBER"); then + echo "::error::could not read issue #${NUMBER}'s project items - an unreadable card is not" \ + "a card that may be left where it is" >&2 + exit 1 + fi + reject_graphql_errors "$IRESP" \ + "the project-items read came back with GraphQL errors - unreadable, not 'no card'" || exit 1 + PI_TOTAL=$(jq -r '.data.repository.issue.projectItems.totalCount // -1' <<< "$IRESP") + # A MISSING connection is not an EMPTY one. `totalCount` is absent when + # the issue itself did not resolve, and retrying that four more times + # then reporting "not on the board" would name the wrong cause. + if [ "${PI_TOTAL:--1}" -lt 0 ]; then + echo "::error::the read did not describe issue #${NUMBER}'s project items at all" \ + "(no totalCount) - an unreadable response is not an empty one" >&2 + exit 1 + fi + NODE=$(jq -c --arg n "$PROJECT_NUMBER" 'first(.data.repository.issue.projectItems.nodes[]? + | select(.project.number == ($n | tonumber))) // {}' <<< "$IRESP") + ITEM_ID=$(jq -r '.id // ""' <<< "$NODE") + if [ -n "$ITEM_ID" ] && [ "$ITEM_ID" != "null" ]; then + CURRENT_COL=$(jq -r '.status.name // ""' <<< "$NODE") + ARCHIVED=$(jq -r 'if .isArchived == true then "true" else "false" end' <<< "$NODE") + break + fi + if [ "${PI_TOTAL:-0}" -gt 20 ]; then + echo "::error::issue #${NUMBER} is on ${PI_TOTAL} projects and the kanban card is not among" \ + "the 20 read - paginate rather than reporting a truncated read as 'not on the board'" >&2 + exit 1 + fi + echo "not on project #${PROJECT_NUMBER} yet (attempt ${attempt}/5) - add-to-kanban may still be running" + if [ "$attempt" -lt 5 ]; then sleep 5; fi + done + + # FAIL CLOSED, LOUDLY, AND THE DIRECTION IS THE DECISION HERE. + # + # The three sibling consumers of the board chose their failure paths as a + # SET (backend#2243), and this one resembles `advance-deploy-env.yml`: no + # fallback, abort. `kanban-closure-router.yml` exits 0 on a card it cannot + # find because declining to write is its conservative end state -- it is + # protecting shipped state from being overwritten. Declining here is not + # conservative: it reproduces the exact defect this job exists to fix, a + # defect parked in `Backlog` that nobody reads. And `kanban-reconcile.yml` + # may skip because it runs weekly and gets another go; a `labeled` event + # fires ONCE, so a green no-op is the last anyone hears of it. + if [ -z "$ITEM_ID" ] || [ "$ITEM_ID" = "null" ]; then + echo "::error::issue #${NUMBER} carries '${BUG_LABEL}' but is not on project" \ + "#${PROJECT_NUMBER} after 5 tries. It needs '${TARGET_COLUMN}' and this run could not" \ + "put it there - check this repo's add-to-kanban caller, then set the column by hand." >&2 + exit 1 + fi + + # ---- 4. may the card move? -------------------------------------- + # Byte-identical to `kanban-closure-router.yml`'s, and asserted so by + # the selftest: both read the same `$PROJ` shape, so a divergence would + # be a defect rather than a difference. + col_index() { + echo "$PROJ" | jq -r --arg s "$1" \ + '[.data.organization.projectV2.fields.nodes[] + | select(.name=="Status") | .options[].name] | index($s) // -1' + } + # selftest:monotonic-start + # MONOTONIC. Automation in this org never moves a card backward, and the + # direction is asked of the BOARD rather than restated here: a rank table + # in this file would agree with itself while disagreeing with reality + # (backend#1729 rule 1). + # + # promote the card is at $SOURCE_COLUMN, or is on the board with no + # Status at all. Those are the only two states from which + # $TARGET_COLUMN is forward. An unplaced card is not "past + # Ready" -- leaving it unplaced keeps it invisible, which is + # the complaint. + # hold anywhere else: `In progress`, `Code review`, `On dev`, + # `FR on staging`, `Ready for prod`, `Prod`, `Done`, + # `Cancelled`, `North Stars`, already `Ready`, or archived. + # The label routinely arrives AFTER triage moved the card, and + # demoting a shipped card would un-ship it on the board. + # unknown a column the board does not report. Nothing can be said + # about "forward" from a position that cannot be placed. + # noboard an anchor is missing, or $TARGET_COLUMN does not sit + # strictly AFTER $SOURCE_COLUMN. The second half is the + # monotonicity assertion itself: position is load-bearing, so + # one drag of the Status options in the UI can make this + # "promotion" a demotion. Checked FIRST, before the + # no-Status shortcut, because a board that cannot be trusted + # to be in pipeline order cannot be trusted for any card + # (backend#1994 is the same hole one file over: existence was + # checked and ORDER was not). + promote_decision() { # $1=the card's current column $2=isArchived + _s=$(col_index "${SOURCE_COLUMN}"); _t=$(col_index "${TARGET_COLUMN}") + if [ "$_s" -lt 0 ] || [ "$_t" -lt 0 ] || [ "$_s" -ge "$_t" ]; then echo noboard; return; fi + if [ "${2:-}" = "true" ]; then echo hold; 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 "$_s" ]; then echo promote; else echo hold; fi + } + # selftest:monotonic-end + + # selftest:policy-start + _d=$(promote_decision "${CURRENT_COL:-}" "${ARCHIVED:-}") + case "$_d" in + promote) + _write=yes ;; + hold) + _write=no + echo "::notice::#${NUMBER} sits in '${CURRENT_COL:-}' (archived=${ARCHIVED:-false})," \ + "not '${SOURCE_COLUMN}' - leaving it alone, automation never moves a card backward" ;; + unknown) + # NOT a quiet decline. The router's `unknown` arm exits 0 because + # there, declining is the safe end state; here it means a card that + # should be in the pull queue is somewhere this job cannot place, + # and nobody would ever hear about it. + echo "::error::#${NUMBER} sits in '${CURRENT_COL:-}', which project #${PROJECT_NUMBER}" \ + "does not report as a Status option - refusing to guess whether '${TARGET_COLUMN}' is forward" >&2 + exit 1 ;; + noboard) + echo "::error::project #${PROJECT_NUMBER} does not place '${SOURCE_COLUMN}' strictly before" \ + "'${TARGET_COLUMN}' in its Status options. Promoting would be a DEMOTION, so nothing was" \ + "written. Check the column order and names on the board." >&2 + exit 1 ;; + *) + # No fall-through. An unrecognised verdict is a code defect, and the + # one thing it must not do is reach the write. + echo "::error::unrecognised promotion verdict '${_d}' - refusing to write" >&2 + exit 1 ;; + esac + # selftest:policy-end + + if [ "$_write" != "yes" ]; then + exit 0 + fi + + # shellcheck disable=SC2016 # the $names are GraphQL variables, not shell - keep literal + if ! WRESP=$(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="$TARGET_OPT"); then + echo "::error::the Status write failed for issue #${NUMBER} - the card is still in" \ + "'${CURRENT_COL:-}' and needs '${TARGET_COLUMN}' by hand" >&2 + exit 1 + fi + # The write goes through the SAME rejection as the two reads. This event + # fires exactly ONCE, so a false success here is permanent: the step logs a + # Backlog -> Ready move, stays green, and the card never moved. + reject_graphql_errors "$WRESP" \ + "the Status write for issue #${NUMBER} came back with GraphQL errors at exit 0 - the card is still in '${CURRENT_COL:-}' and needs '${TARGET_COLUMN}' by hand" || exit 1 + # Absence of errors is not presence of the write. Confirm the mutation + # returned the item it claims to have moved -- the same "did it actually + # land" discipline merge-confirm.sh applies to a merge. + if [ -z "$(jq -r '.data.updateProjectV2ItemFieldValue.projectV2Item.id // empty' <<< "$WRESP")" ]; then + echo "::error::the Status write for issue #${NUMBER} returned no item id, so the move is UNCONFIRMED - the card needs '${TARGET_COLUMN}' by hand" >&2 + exit 1 + fi + echo "-> issue #${NUMBER}: Status '${CURRENT_COL:-}' -> '${TARGET_COLUMN}'" + { + echo "### Defect skipped refinement" + echo + echo "\`${BUG_LABEL}\` on issue #${NUMBER}: \`${CURRENT_COL:-}\` -> \`${TARGET_COLUMN}\`" + } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/kanban-columns.yml b/.github/workflows/kanban-columns.yml index adb27c1..0e6f0f7 100644 --- a/.github/workflows/kanban-columns.yml +++ b/.github/workflows/kanban-columns.yml @@ -29,6 +29,9 @@ on: # ever checked by the daily cron. - ".github/workflows/kanban-archive.yml" - ".github/workflows/wip-limit-check.yml" + # backend#2348: its bug-to-ready job names `Backlog` and `Ready`, so a PR + # touching it must run this check rather than waiting for the daily cron. + - ".github/workflows/customer-priority-bump.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/CLAUDE.md b/CLAUDE.md index 657d2ed..01dd965 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,7 +36,7 @@ and this repo's default reviewer. Rollout: tracebloc/backend#1602. ### Engineer kanban -- Every ticket on the board carries a `Status` — no card sits at "No Status". New tickets start in `Backlog`. **Bugs are the exception:** label them `work-type:bug` (the Bug template does it) and put them straight into `Ready` — defects don't wait for refinement. +- Every ticket on the board carries a `Status` — no card sits at "No Status". New tickets start in `Backlog`. **Bugs are the exception:** label them `work-type:bug` (the Bug template does it) and automation moves the card straight into `Ready` — defects don't wait for refinement. Three repos aren't wired for the label trigger yet (`.github`, `release-train`, `rfcs`); move the card yourself there. - Picking up work: the team coordinates. `Ready` is the refined queue — bugs excepted, per the line above — and the first choice when it's stocked; pulling from `Backlog` is normal when refinement hasn't caught up — say what you're taking. - Merging to `develop` moves the card to `On dev` automatically; there is no dev-side review. - Functional review happens once, on staging: when it passes, comment `/fr-pass` on the PR or drag the card to `Ready for prod`. Self-signoff is allowed. diff --git a/Makefile b/Makefile index e762cf4..beef90a 100644 --- a/Makefile +++ b/Makefile @@ -275,7 +275,7 @@ SELFTEST_FILES := $(sort $(wildcard scripts/tests/*-selftest.py scripts/tests/*- # and an invisible file makes the coverage assertion pass vacuously. MUTATION_FILES := $(sort $(wildcard scripts/tests/*-mutations.py)) MUTATION_TARGETS := mutation-house-rules mutation-pipefail-early-close \ - mutation-bugbot-gate + mutation-bugbot-gate mutation-bug-to-ready # THE WHOLE MUTATION TIER, BY NAME OF THE LIST. Every entry point -- CI, # `check-all`, `lint` -- depends on one of these two rather than on any @@ -306,7 +306,8 @@ SELFTEST_TARGETS := selftest-caller-drift selftest-blocked-marker selftest-stand selftest-kanban-deploy-state selftest-git-reap \ selftest-mint-scope selftest-house-rules \ selftest-pipefail-early-close \ - selftest-bugbot-gate + selftest-bugbot-gate \ + selftest-bug-to-ready selftests: selftests-cover $(SELFTEST_TARGETS) @@ -490,6 +491,26 @@ mutation-bugbot-gate: mutation-bugbot-gate-dry: $(PYTHON) scripts/tests/bugbot-gate-mutations.py --dry +# The bug-label promotion (backend#2348). guard-pyyaml: the suite parses THREE +# workflows -- it extracts the decision out of `customer-priority-bump.yml` by its +# `# selftest:` markers, asserts `col_index` byte-identical to the router's, and +# derives the board's Status vocabulary from advance-deploy-env's `rank()`. +# selftests.yml installs PyYAML, which is where CI runs this. +.PHONY: selftest-bug-to-ready +selftest-bug-to-ready: guard-pyyaml + $(PYTHON) scripts/tests/bug-to-ready-selftest.py + +# NOT in `selftests`: each mutation re-runs the suite. Measured on a laptop: the +# suite alone ~1.5s, the full 21-mutation pass ~30s. `--dry` resolves every anchor +# in milliseconds and rides `lint`, which catches the way this really breaks -- a +# refactor moving a line an anchor matched on -- without proving the cases catch. +.PHONY: mutation-bug-to-ready mutation-bug-to-ready-dry +mutation-bug-to-ready: guard-pyyaml + $(PYTHON) scripts/tests/bug-to-ready-mutations.py + +mutation-bug-to-ready-dry: guard-pyyaml + $(PYTHON) scripts/tests/bug-to-ready-mutations.py --dry + .PHONY: selftest-git-reap selftest-git-reap: bash scripts/tests/git-reap-selftest.sh diff --git a/org-standards.md b/org-standards.md index 0679ab7..7ef0b1e 100644 --- a/org-standards.md +++ b/org-standards.md @@ -31,7 +31,7 @@ ### Engineer kanban -- Every ticket on the board carries a `Status` — no card sits at "No Status". New tickets start in `Backlog`. **Bugs are the exception:** label them `work-type:bug` (the Bug template does it) and put them straight into `Ready` — defects don't wait for refinement. +- Every ticket on the board carries a `Status` — no card sits at "No Status". New tickets start in `Backlog`. **Bugs are the exception:** label them `work-type:bug` (the Bug template does it) and automation moves the card straight into `Ready` — defects don't wait for refinement. Three repos aren't wired for the label trigger yet (`.github`, `release-train`, `rfcs`); move the card yourself there. - Picking up work: the team coordinates. `Ready` is the refined queue — bugs excepted, per the line above — and the first choice when it's stocked; pulling from `Backlog` is normal when refinement hasn't caught up — say what you're taking. - Merging to `develop` moves the card to `On dev` automatically; there is no dev-side review. - Functional review happens once, on staging: when it passes, comment `/fr-pass` on the PR or drag the card to `Ready for prod`. Self-signoff is allowed. diff --git a/scripts/kanban-columns-check.py b/scripts/kanban-columns-check.py index 858aea5..deb8de8 100755 --- a/scripts/kanban-columns-check.py +++ b/scripts/kanban-columns-check.py @@ -89,6 +89,14 @@ # because the paths-filter assertion and the idiom cross-check both key on it. "kanban-archive.yml", "wip-limit-check.yml", + # backend#2348. Its `bug-to-ready` job names BOTH anchors it promotes between + # -- `SOURCE_COLUMN: "Backlog"` and `TARGET_COLUMN: "Ready"` -- and a rename in + # the Projects UI would turn the promotion into a silent no-op: the option id + # is resolved from the board by name, so a missing name resolves to nothing. + # `unlisted_namers()` would have found this file anyway; listing it is what + # puts those two names under the board assertion instead of merely reporting + # that they are unchecked. + "customer-priority-bump.yml", ) diff --git a/scripts/tests/bug-to-ready-mutations.py b/scripts/tests/bug-to-ready-mutations.py new file mode 100644 index 0000000..a93115b --- /dev/null +++ b/scripts/tests/bug-to-ready-mutations.py @@ -0,0 +1,251 @@ +#!/usr/bin/env python3 +"""Mutation harness for the bug-label promotion (tracebloc/backend#2348). + +`bug-to-ready-selftest.py` asserts the job's behaviour; this asserts the +SELFTEST. Break a rule in the real artefact, watch the suite redden, restore. A +case that stays green under its own rule being deleted is vacuous, and a green +log cannot tell you which of the suite's assertions are load-bearing. + +THE MUTATION EDITS THE CODE UNDER TEST (CLAUDE.md rule 9). Every anchor below +lands in `.github/workflows/customer-priority-bump.yml` or in `org-standards.md` +-- the two files the suite reads -- and the suite is then re-run against them. +There is no second copy of the decision anywhere in here. The alternative shape, +re-implementing the rule inline and mutating the copy, is indistinguishable from +real coverage in a log and has bitten this org twice (.github#114, #115). + +EVERY ANCHOR MUST MATCH EXACTLY ONCE. An anchor that matches twice mutates an +arbitrary one, so an "uncaught" verdict is about the wrong line; an anchor that +matches zero times is stale and fails the run exactly like an uncaught mutation. +That is the assertion that the anchor ACTUALLY APPLIED -- otherwise an inert +mutation and good coverage look identical (CLAUDE.md rule 5). `--dry` resolves +every anchor without running the suite, which is what belongs in the fast tier: +it catches the way this file really breaks, a refactor moving a line an anchor +matched on. + + bug-to-ready-mutations.py run them all + bug-to-ready-mutations.py --dry resolve anchors only + +WHAT IS DELIBERATELY NOT MUTATED, stated because an unstated gap is how a suite +comes to be trusted for more than it proves: the network seam. The board read, +the 5x5s retry, the "card is not on the board" fail-closed and the +`updateProjectV2ItemFieldValue` write's TRANSPORT are driven by no case, so a +mutation to them would report UNCAUGHT for a reason that is about the suite's scope rather +than about its rigour. Those paths are asserted by inspection and by copying the +shape of the sibling workflows; they are named in the PR body as uncovered. +WHAT IS NOW COVERED, and was not: the write's `errors[]` handling. It shares one +`reject_graphql_errors` with both reads, that function is extracted by name, and +the two anchors at the end of this list break each of its arms. +""" +import os +import subprocess +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +WF = ROOT / ".github" / "workflows" / "customer-priority-bump.yml" +CANON = ROOT / "org-standards.md" +SUITE = ROOT / "scripts" / "tests" / "bug-to-ready-selftest.py" + +# (label, file, old, new) +MUTATIONS = [ + # --- the monotonic guard ------------------------------------------------ + ("the monotonic guard: promote from ANY placeable column, not just the source", + WF, + ' if [ "$_c" -eq "$_s" ]; then echo promote; else echo hold; fi', + ' if [ "$_c" -ge 0 ]; then echo promote; else echo hold; fi'), + ("the anchor ORDER half: check existence only, not that the target follows the source", + WF, + 'if [ "$_s" -lt 0 ] || [ "$_t" -lt 0 ] || [ "$_s" -ge "$_t" ]; then echo noboard; return; fi', + 'if [ "$_s" -lt 0 ] || [ "$_t" -lt 0 ]; then echo noboard; return; fi'), + # The ORDER of the two checks is itself load-bearing: with the shortcut first, + # an unplaced card on an inverted board is promoted by a board nothing could + # place it on. + ("the anchor check runs SECOND, after the no-Status shortcut", + WF, + ' _s=$(col_index "${SOURCE_COLUMN}"); _t=$(col_index "${TARGET_COLUMN}")\n' + ' if [ "$_s" -lt 0 ] || [ "$_t" -lt 0 ] || [ "$_s" -ge "$_t" ]; then echo noboard; return; fi\n' + ' if [ "${2:-}" = "true" ]; then echo hold; return; fi\n' + ' case "${1:-}" in\n' + ' ""|"No status") echo promote; return ;;\n' + ' esac\n', + ' case "${1:-}" in\n' + ' ""|"No status") echo promote; return ;;\n' + ' esac\n' + ' _s=$(col_index "${SOURCE_COLUMN}"); _t=$(col_index "${TARGET_COLUMN}")\n' + ' if [ "$_s" -lt 0 ] || [ "$_t" -lt 0 ] || [ "$_s" -ge "$_t" ]; then echo noboard; return; fi\n' + ' if [ "${2:-}" = "true" ]; then echo hold; return; fi\n'), + ("the unplaceable column falls open and is promoted", + WF, + ' if [ "$_c" -lt 0 ]; then echo unknown; return; fi', + ' if [ "$_c" -lt 0 ]; then echo promote; return; fi'), + ("an ARCHIVED card is promoted like a live one", + WF, + ' if [ "${2:-}" = "true" ]; then echo hold; return; fi', + ' if [ "${2:-}" = "__never__" ]; then echo hold; return; fi'), + ("a card with NO Status is left unplaced, which is what kept the defect invisible", + WF, + ' ""|"No status") echo promote; return ;;', + ' ""|"No status") echo hold; return ;;'), + # The absence sentinel. `// 0` makes every absent column read as position 0, + # which is the source column's own index. + ("col_index reports an ABSENT column as position 0 instead of -1", + WF, + '| index($s) // -1', + '| index($s) // 0'), + + # --- the policy: the fail-closed DIRECTION this job chose --------------- + ("noboard exits 0, so an unreadable board order becomes a green no-op", + WF, + '"written. Check the column order and names on the board." >&2\n' + ' exit 1 ;;', + '"written. Check the column order and names on the board." >&2\n' + ' exit 0 ;;'), + ("unknown exits 0, the router's direction rather than this job's", + WF, + '"does not report as a Status option - refusing to guess whether \'${TARGET_COLUMN}\' is forward" >&2\n' + ' exit 1 ;;', + '"does not report as a Status option - refusing to guess whether \'${TARGET_COLUMN}\' is forward" >&2\n' + ' exit 0 ;;'), + ("the case falls through to the write on an unrecognised verdict", + WF, + ' echo "::error::unrecognised promotion verdict \'${_d}\' - refusing to write" >&2\n' + ' exit 1 ;;', + ' _write=yes ;;'), + + # --- the label/event gate ---------------------------------------------- + ("the label is matched as a SUBSTRING, so 'work-type:bugfix' promotes", + WF, + ' if [ "$2" != "$3" ]; then echo "refuse:other-label"; return; fi', + ' case "$2" in *"$3"*) ;; *) echo "refuse:other-label"; return ;; esac'), + ("an unreadable label reads as 'some other label' instead of failing closed", + WF, + ' if [ -z "${2:-}" ]; then echo "refuse:unreadable-label"; return; fi', + ' if [ -z "${2:-x}" ]; then echo "refuse:unreadable-label"; return; fi'), + ("an empty configured label matches everything instead of refusing", + WF, + ' if [ -z "${3:-}" ]; then echo "refuse:no-configured-label"; return; fi', + ' if [ -z "${3:-x}" ]; then echo "refuse:no-configured-label"; return; fi'), + ("a pull_request payload is accepted", + WF, + ' if [ "${4:-}" = "true" ]; then echo "refuse:pull-request-payload"; return; fi', + ' if [ "${4:-}" = "__never__" ]; then echo "refuse:pull-request-payload"; return; fi'), + ("any event may promote, not only `issues`", + WF, + ' if [ "${1:-}" != "issues" ]; then echo "refuse:not-an-issues-event"; return; fi', + ' if [ "${1:-}" = "__never__" ]; then echo "refuse:not-an-issues-event"; return; fi'), + + # --- the job `if:` and the mint ---------------------------------------- + ("the job `if:` becomes a contains(), so the cost gate is LOOSER than the decision", + WF, + " if: github.event_name == 'issues' && github.event.label.name == inputs.bug-label", + " if: github.event_name == 'issues' && contains(github.event.label.name, inputs.bug-label)"), + ("the job `if:` stops pinning the event name", + WF, + " if: github.event_name == 'issues' && github.event.label.name == inputs.bug-label", + " if: github.event.label.name == inputs.bug-label"), + ("the new mint drops its scopes and takes the App's full grant", + WF, + " permission-issues: read\n permission-organization-projects: write\n", + ""), + + # --- the vocabulary is really derived from the canon -------------------- + # Both directions, because a derivation is only live if BOTH sides moving is + # a finding: the workflow drifting from the rule, and the rule being reworded + # out from under the workflow. + ("the workflow's label default drifts from the written rule", + WF, + ' default: "work-type:bug"', + ' default: "work-type:bugs"'), + ("the workflow writes a different column than the rule names", + WF, + ' TARGET_COLUMN: "Ready"', + ' TARGET_COLUMN: "Ready for prod"'), + ("the CANON renames the label and the workflow is not updated with it", + CANON, + "label them `work-type:bug`", + "label them `type:bug`"), + # --- the shared GraphQL `errors[]` rejection (Bugbot on .github#313) ------ + # Both reads AND the write go through one function now. These two anchors are + # what stop that function silently losing an arm. + ("the unparseable-JSON arm: treat a body jq cannot read as error-free", + WF, + ' if ! jq -e . >/dev/null 2>&1 <<< "$1"; then', + ' if false; then'), + ("the errors[] arm: stop rejecting an errors[] payload at exit 0", + WF, + ' if jq -e \'has("errors")\' <<< "$1" >/dev/null 2>&1; then', + ' if false; then'), +] + + +def apply_one(src, old, new): + n = src.count(old) + if n != 1: + raise LookupError("anchor matched %d times, expected exactly 1: %r" + % (n, old[:90])) + out = src.replace(old, new, 1) + return None if out == src else out + + +def main(): + dry = "--dry" in sys.argv + pristine = {p: p.read_text(encoding="utf-8") for p in (WF, CANON)} + stale, uncaught = [], [] + + for label, path, old, new in MUTATIONS: + try: + mutated = apply_one(pristine[path], old, new) + except LookupError as exc: + stale.append((label, str(exc))) + continue + if mutated is None: + stale.append((label, "NO-OP: the mutation changed nothing")) + continue + if dry: + print(" anchor ok %s" % label) + continue + path.write_text(mutated, encoding="utf-8") + try: + run = subprocess.run( + [sys.executable, "-B", str(SUITE)], + capture_output=True, text=True, cwd=str(ROOT), + env={**os.environ, "PYTHONDONTWRITEBYTECODE": "1"}, + ) + finally: + # ALWAYS restore, including on a crash. A mutation left on disk makes + # every later run measure the wrong file, and the tell is a suite that + # reddens for reasons nobody typed. + path.write_text(pristine[path], encoding="utf-8") + caught = [ln.strip()[5:].strip() for ln in run.stdout.splitlines() + if ln.strip().startswith("FAIL:")] + # A crash counts as caught ONLY if the suite actually ran and reported. A + # bare traceback, or the extractor's own `sys.exit`, means the mutation + # broke the harness rather than being detected by a case -- which is not + # coverage, and must not be logged as if it were. + reported = "bug-to-ready-selftest:" in run.stdout + if reported and run.returncode != 0: + print(" caught %s\n by: %s" % (label, "; ".join(caught)[:150])) + elif not reported: + uncaught.append((label, "the suite did not report -- mutation broke the harness")) + print(" UNCAUGHT %s (harness broke, not detected)" % label) + else: + uncaught.append((label, "the suite passed with this broken")) + print(" UNCAUGHT %s" % label) + + for path, text in pristine.items(): + if path.read_text(encoding="utf-8") != text: + sys.stderr.write("::error::%s was left mutated. Restore it from git.\n" % path.name) + return 2 + + print("\n%d mutation(s): %d stale, %d uncaught" % (len(MUTATIONS), len(stale), len(uncaught))) + for label, why in stale: + sys.stderr.write("::error::STALE mutation `%s`: %s\n" % (label, why)) + for label, why in uncaught: + sys.stderr.write( + "::error::UNCAUGHT `%s`: %s. Add a case that fails under it, or delete " + "the mutation and say why it is not worth pinning.\n" % (label, why)) + return 1 if (stale or uncaught) else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/tests/bug-to-ready-selftest.py b/scripts/tests/bug-to-ready-selftest.py new file mode 100644 index 0000000..524794e --- /dev/null +++ b/scripts/tests/bug-to-ready-selftest.py @@ -0,0 +1,430 @@ +#!/usr/bin/env python3 +"""The bug-label promotion is read OUT of the workflow and exercised. + +WHY THIS EXISTS (tracebloc/backend#2348) + +The org rule -- "bugs get `work-type:bug` and go straight into `Ready` (defects +skip refinement)" -- was documented in `org-standards.md` and implemented +nowhere. `customer-priority-bump.yml`'s `bug-to-ready` job implements it, and +this asserts the two things that job can get catastrophically wrong: + + * it moves a card that should not move (un-shipping work on the board), or + * it reports success without having moved anything (the defect it fixes). + +WHY IT IS EXTRACTED RATHER THAN COPIED + +`customer-priority-bump.yml` is a REUSABLE workflow: it runs in the CALLER's +checkout, so no script in this repo is on disk for it and the decision has to be +inline shell. A copy of that shell in here would let the workflow drift while +this file stayed green -- the same defect class the rule itself had. So every +piece under test is pulled out of the YAML by its `# selftest:` markers and run +verbatim (CLAUDE.md rule 9). If someone renames or reshapes a region, this test +stops finding it and fails loudly rather than testing a stale duplicate. + +WHERE THE VOCABULARY COMES FROM (CLAUDE.md rules 1 and 6) + +A monotonicity check that tries two columns is vacuous, and a hand-written list +of the other ten agrees with itself. So the board's Status vocabulary is DERIVED +from `advance-deploy-env.yml`'s `rank()` -- the org's declared pipeline order, +and the same file this job's monotonicity was modelled on -- and cross-checked +against `kanban-deploy-state-selftest.py`'s independently written BOARD. Two +derivations that disagree is a finding, not a tie to break here. + +The LABEL and the two COLUMN NAMES come from `org-standards.md`, which is the +canon the rule is written in. Rename the label there and this reddens, which is +the point: the workflow's default and the written rule cannot drift apart. + +Exit 0 when every case behaves as specified. +""" +from __future__ import annotations + +import ast +import json +import os +import re +import shlex +import subprocess +import sys + +import yaml + +HERE = os.path.dirname(os.path.abspath(__file__)) +ROOT = os.path.join(HERE, os.pardir, os.pardir) +WORKFLOWS = os.path.join(ROOT, ".github", "workflows") + +BUG_WF = os.path.join(WORKFLOWS, "customer-priority-bump.yml") +ROUTER = os.path.join(WORKFLOWS, "kanban-closure-router.yml") +ADVANCE = os.path.join(WORKFLOWS, "advance-deploy-env.yml") +SIBLING_SUITE = os.path.join(HERE, "kanban-deploy-state-selftest.py") +CANON = os.path.join(ROOT, "org-standards.md") + +JOB = "bug-to-ready" + +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: + """Strip the workflow's indentation so the block parses standalone.""" + 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 doc(path): + with open(path, encoding="utf-8") as fh: + return yaml.safe_load(fh) + + +def runs(path, job=None) -> "list[str]": + d = doc(path) + jobs = d["jobs"] if job is None else {job: d["jobs"][job]} + return [s["run"] for j in jobs.values() + for s in j.get("steps", []) if "run" in s] + + +def extract(path, pattern: str, what: str, job=None) -> str: + for body in runs(path, job): + m = re.search(pattern, body, re.S | re.M) + if m: + return dedent(m.group(0)) + # FAIL CLOSED. A missing region is "cannot tell", never "nothing to check": + # falling back to a copy is how a suite comes to prove a regex nothing uses. + sys.exit(f"could not find {what} in {os.path.basename(path)} - was it renamed " + "or its markers dropped? This test refuses to fall back to a copy.") + + +def func(path, name: str, job=None) -> str: + return extract(path, rf"^\s*{name}\(\) \{{.*?^\s*\}}\s*$", f"{name}()", job) + + +def region(path, marker: str, job=None) -> str: + return extract(path, rf"^[ \t]*# selftest:{marker}-start\b.*?^[ \t]*# selftest:" + rf"{marker}-end\b[^\n]*$", f"the # selftest:{marker}-* region", + job) + + +def sh(script: str) -> "tuple[int, str]": + out = subprocess.run(["bash", "-c", script], capture_output=True, text=True) + return out.returncode, (out.stdout + out.stderr).strip() + + +# --------------------------------------------------------------------------- +# 0. THE VOCABULARY, DERIVED TWICE. +# --------------------------------------------------------------------------- +# `rank()` in advance-deploy-env.yml is the org's declared pipeline order and the +# construct this job's monotonicity was modelled on. Parsing it gives both the +# names and their order; a hand-written list here would be a third copy. +RANK_ARM = re.compile(r'^\s*"([^"]+)"\)\s*echo\s+(\d+)\s*;;', re.M) +_rank_src = func(ADVANCE, "rank") +_arms = [(name, int(n)) for name, n in RANK_ARM.findall(_rank_src)] +if len(_arms) < 5: + sys.exit(f"parsed only {len(_arms)} rank() arms out of advance-deploy-env.yml - " + "the pattern is stale, and a vocabulary of 0 would make every case " + "below pass vacuously") +# Sorted by declared rank, ties (Done/Cancelled share a rank) in file order. +BOARD = [name for name, _ in sorted(_arms, key=lambda kv: (kv[1], _arms.index(kv)))] + +# The second, independent declaration: the sibling suite's BOARD literal, written +# by hand from a live read of project #2. Two derivations that disagree means one +# of them is stale, and this file is not the place to pick a winner. +with open(SIBLING_SUITE, encoding="utf-8") as fh: + _m = re.search(r"^BOARD = (\[[^\]]*\])", fh.read(), re.M | re.S) +if _m is None: + sys.exit("could not find BOARD in kanban-deploy-state-selftest.py - the " + "cross-check cannot be made, so this suite refuses to report on a " + "single unchecked derivation") +SIBLING_BOARD = ast.literal_eval(_m.group(1)) +record(sorted(BOARD) == sorted(SIBLING_BOARD), + "the Status vocabulary derived from rank() agrees with the sibling suite's board", + f"{len(BOARD)} columns: {', '.join(BOARD)}") +record(BOARD == SIBLING_BOARD, + "and in the same ORDER, which is what the monotonic gate is derived from", + f"rank(): {BOARD}\n sibling: {SIBLING_BOARD}") + +# --------------------------------------------------------------------------- +# 1. THE RULE'S OWN WORDS decide the label and the two anchors. +# --------------------------------------------------------------------------- +with open(CANON, encoding="utf-8") as fh: + CANON_TEXT = fh.read() + + +def from_canon(pattern: str, what: str) -> str: + m = re.search(pattern, CANON_TEXT) + if m is None: + sys.exit(f"could not read {what} out of org-standards.md (pattern " + f"{pattern!r}). The workflow's value cannot be checked against " + "the written rule, so this suite fails rather than assuming.") + return m.group(1) + + +CANON_LABEL = from_canon(r"label them `([^`]+)`", "the bug label") +CANON_SOURCE = from_canon(r"New tickets start in `([^`]+)`", "the starting column") +CANON_TARGET = from_canon(r"straight into `([^`]+)`", "the target column") + +_bug_job = doc(BUG_WF)["jobs"][JOB] +_step = [s for s in _bug_job["steps"] if "run" in s][0] +_env = _step.get("env") or {} +_inputs = doc(BUG_WF)[True]["workflow_call"]["inputs"] # `on:` parses as True + +record(_inputs["bug-label"]["default"] == CANON_LABEL, + "the bug-label default is the label org-standards.md names", + f"workflow: {_inputs['bug-label']['default']!r}; canon: {CANON_LABEL!r}") +record(_env.get("SOURCE_COLUMN") == CANON_SOURCE, + "SOURCE_COLUMN is the column org-standards.md says new tickets start in", + f"workflow: {_env.get('SOURCE_COLUMN')!r}; canon: {CANON_SOURCE!r}") +record(_env.get("TARGET_COLUMN") == CANON_TARGET, + "TARGET_COLUMN is the column org-standards.md sends defects to", + f"workflow: {_env.get('TARGET_COLUMN')!r}; canon: {CANON_TARGET!r}") +record(CANON_SOURCE in BOARD and CANON_TARGET in BOARD + and BOARD.index(CANON_SOURCE) < BOARD.index(CANON_TARGET), + "the canon's two columns exist in the declared order, source before target", + f"{CANON_SOURCE} @{BOARD.index(CANON_SOURCE) if CANON_SOURCE in BOARD else '?'} " + f"-> {CANON_TARGET} @{BOARD.index(CANON_TARGET) if CANON_TARGET in BOARD else '?'}") + +# --------------------------------------------------------------------------- +# 2. THE JOB `if:` IS A COST GATE, AND MUST BE EXACT. +# --------------------------------------------------------------------------- +# The workflow says the `if:` can only ever be STRICTER than `label_gate`. A +# `contains()` there would make it LOOSER -- `work-type:bugfix` would mint a +# token and reach a gate that then refuses it -- and a substring match is the +# classic way this shape goes wrong, so it is asserted rather than trusted. +_if = str(_bug_job.get("if", "")) +record("contains(" not in _if and "inputs.bug-label" in _if + and "github.event.label.name == inputs.bug-label" in _if, + "the job `if:` is exact equality against inputs.bug-label", + f"if: {_if}") +record("github.event_name == 'issues'" in _if, + "the job `if:` also pins the event to `issues`", + "a pull_request payload must not reach the promotion at all") + +# The mint must be scoped (backend#2157): this job is new, so there is no +# pre-existing full grant to inherit and no reason to take one. +_mint = [s for s in _bug_job["steps"] + if str(s.get("uses", "")).startswith("actions/create-github-app-token")] +record(len(_mint) == 1 and any(k.startswith("permission-") + for k in (_mint[0].get("with") or {})), + "the token mint names explicit permission-* scopes", + f"with: {sorted((_mint[0].get('with') or {})) if _mint else ''}") + +# --------------------------------------------------------------------------- +# 3. THE INDEX PRIMITIVE IS SHARED WITH THE ROUTER, byte for byte. +# --------------------------------------------------------------------------- +# Both read the same `$PROJ` response shape, so a divergence between them is a +# defect rather than a difference. If one ever legitimately has to differ, this +# assertion is where that gets argued. +COL_INDEX = func(BUG_WF, "col_index", JOB) +record(COL_INDEX == func(ROUTER, "col_index"), + "col_index() is byte-identical to kanban-closure-router.yml's", + "same $PROJ shape, same primitive -- one definition, two callers") + +MONOTONIC = region(BUG_WF, "monotonic", JOB) +POLICY = region(BUG_WF, "policy", JOB) +LABEL_GATE = region(BUG_WF, "label-gate", JOB) + + +def proj(names) -> str: + return json.dumps({"data": {"organization": {"projectV2": {"fields": { + "nodes": [{"name": "Status", + "options": [{"name": n} for n in names]}]}}}}}) + + +def decide(current: str, archived: str = "false", names=None, + source=None, target=None) -> str: + """Run the workflow's own promote_decision against a synthetic board.""" + rc, out = sh(f""" +set -euo pipefail +PROJ={shlex.quote(proj(BOARD if names is None else names))} +SOURCE_COLUMN={shlex.quote(source or CANON_SOURCE)} +TARGET_COLUMN={shlex.quote(target or CANON_TARGET)} +{COL_INDEX} +{MONOTONIC} +promote_decision {shlex.quote(current)} {shlex.quote(archived)} +""") + return out if rc == 0 else f"ERROR({rc}): {out}" + + +# --------------------------------------------------------------------------- +# 4. EVERY COLUMN IN THE VOCABULARY, not the two that are convenient. +# --------------------------------------------------------------------------- +# Mutation coverage cannot see a vocabulary gap (CLAUDE.md rule 6): a gate that +# promotes from `North Stars` too passes every two-column test ever written. +_promoting = [c for c in BOARD if decide(c) == "promote"] +record(_promoting == [CANON_SOURCE], + f"of all {len(BOARD)} declared columns, exactly {CANON_SOURCE!r} promotes", + f"promoting: {_promoting}; every other column must hold") +for col in BOARD: + want = "promote" if col == CANON_SOURCE else "hold" + got = decide(col) + record(got == want, f"a card at {col!r} -> {want}", f"-> {got}") + +# A card on the board with no Status is not "past Ready" -- leaving it unplaced +# is what keeps a defect invisible, which is the whole complaint. +for col in ("", "No status"): + got = decide(col) + record(got == "promote", f"{col or ''!r} is promoted, not left unplaced", + f"-> {got}") + +# ARCHIVED is out of the flow: the archiver only touches terminal columns, and an +# archived item's field write errors anyway. +record(decide(CANON_SOURCE, archived="true") == "hold", + "an ARCHIVED card holds even when it sits in the source column", + f"-> {decide(CANON_SOURCE, archived='true')}") + +# UNKNOWN MUST NOT FALL OPEN. Nothing can be said about "forward" from a position +# the board does not report. +got = decide("Some Column Nobody Declared") +record(got == "unknown", "a column the board does not report is unknown, not promotable", + f"-> {got}") + +# --------------------------------------------------------------------------- +# 5. THE ANCHORS: missing, and INVERTED. +# --------------------------------------------------------------------------- +for missing in (CANON_SOURCE, CANON_TARGET): + board = [n for n in BOARD if n != missing] + got = decide(CANON_SOURCE, names=board) + record(got == "noboard", f"a board with no {missing!r} column refuses rather than guessing", + f"-> {got}") + +# INVERTED, which is one drag of the Status options away. With `Ready` before +# `Backlog` this job's "promotion" is a demotion, so it must refuse -- and it must +# refuse for the card AT the source column, the one case that would actually +# perform the demotion if the anchor check ran second. +s_i, t_i = BOARD.index(CANON_SOURCE), BOARD.index(CANON_TARGET) +inverted = list(BOARD) +inverted[s_i], inverted[t_i] = inverted[t_i], inverted[s_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(CANON_TARGET) < inverted.index(CANON_SOURCE), inverted +assert sorted(inverted) == sorted(BOARD), inverted +for col in (CANON_SOURCE, CANON_TARGET, "In progress", ""): + got = decide(col, names=inverted) + record(got == "noboard", + f"{col or ''!r} on a board whose anchors are INVERTED refuses", + f"-> {got}; an unsatisfiable order must fail closed, and that includes " + "the unplaced card the shortcut would otherwise let through") + +# --------------------------------------------------------------------------- +# 6. THE POLICY, run verbatim, driven through every verdict. +# --------------------------------------------------------------------------- +# What the job DOES with a verdict is the other half of the decision, and the +# fail-closed DIRECTION is the part this job chose differently from its siblings: +# `unknown` and `noboard` are RED here, where the router exits 0. A cron gets +# another go next week; a `labeled` event fires once. +# +# `_bogus` is not a verdict any board can produce -- it drives the `*)` arm, which +# exists so the `case` has no fall-through to the write. +POLICY_EXPECTED = { + "promote": (0, "_write=yes"), + "hold": (0, "_write=no"), + "unknown": (1, "::error::"), + "noboard": (1, "::error::"), + "_bogus": (1, "::error::"), +} +for verdict, (want_rc, want) in POLICY_EXPECTED.items(): + rc, out = sh(f""" +set -euo pipefail +NUMBER=1; CURRENT_COL='Some Column'; ARCHIVED=false +PROJECT_NUMBER=2; SOURCE_COLUMN={shlex.quote(CANON_SOURCE)}; TARGET_COLUMN={shlex.quote(CANON_TARGET)} +promote_decision() {{ echo {shlex.quote(verdict)}; }} +{POLICY} +echo "_write=${{_write:-unset}}" +""") + record(rc == want_rc and want in out, f"policy: {verdict} -> {want} (rc {want_rc})", + f"rc={rc}; {out.splitlines()[0] if out else ''}") + +# --------------------------------------------------------------------------- +# 7. THE LABEL/EVENT GATE, and each refusal named (CLAUDE.md rule 10). +# --------------------------------------------------------------------------- +# A case that accepts any non-zero exit cannot say WHICH refusal it exercised, so +# every row below pins the specific reason string. +def gate(event="issues", label=None, want=None, pr="false", number="7") -> "tuple[int, str]": + return sh(f""" +set -euo pipefail +EVENT_NAME={shlex.quote(event)} +LABEL_ADDED={shlex.quote(CANON_LABEL if label is None else label)} +BUG_LABEL={shlex.quote(CANON_LABEL if want is None else want)} +HAS_PR_PAYLOAD={shlex.quote(pr)} +NUMBER={shlex.quote(number)} +{LABEL_GATE} +echo "reached-the-board-read" +""") + + +GATE_CASES = [ + ("the exact label on an issue proceeds", {}, 0, "reached-the-board-read"), + # GREEN, because it is the common case: this workflow fires on every + # `labeled` event in 16 repos. + ("a different label is a quiet no-op, not a failure", + {"label": "work-type:docs"}, 0, "nothing to do"), + # The substring trap, in both directions. + ("'work-type:bugfix' is not 'work-type:bug'", + {"label": CANON_LABEL + "fix"}, 0, "nothing to do"), + ("a label the bug label is a prefix OF does not match", + {"label": CANON_LABEL[:-1]}, 0, "nothing to do"), + ("a pull_request payload is refused loudly", + {"pr": "true"}, 1, "refuse:pull-request-payload"), + ("a non-issues event is refused loudly", + {"event": "pull_request"}, 1, "refuse:not-an-issues-event"), + ("an unreadable/absent label in the payload fails rather than reading as 'no'", + {"label": ""}, 1, "refuse:unreadable-label"), + ("an empty configured label refuses instead of matching everything", + {"want": ""}, 1, "refuse:no-configured-label"), +] +for name, kwargs, want_rc, needle in GATE_CASES: + rc, out = gate(**kwargs) + record(rc == want_rc and needle in out, f"label gate: {name}", + f"rc={rc} (want {want_rc}); looked for {needle!r} in: " + f"{out.splitlines()[-1] if out else ''}") + +# --------------------------------------------------------------------------- +# 5. THE GraphQL `errors[]` REJECTION, shared by both reads AND the write. +# --------------------------------------------------------------------------- +# `gh api graphql` exits 0 on an HTTP 200 that carries a GraphQL `errors[]` +# payload. The two reads always rejected that; the WRITE did not -- it discarded +# its response and trusted the exit code, so the job could log a Backlog -> Ready +# move and stay green while the card never moved. On a `labeled` event, which +# fires exactly once, that false success is permanent. (Bugbot on .github#313.) +# +# The function is extracted BY NAME from the workflow, so this assertion and the +# mutation both drive the code the job runs -- not a copy of it (rule 9). +_REJECT = func(BUG_WF, "reject_graphql_errors") + + +def reject(payload: str) -> "tuple[int, str]": + return sh(_REJECT + f'\nreject_graphql_errors {payload!r} "REFUSED-HERE"\n') + + +# Rule 6: the input domain is every shape a GraphQL response can arrive in, not +# just the happy one and the obvious sad one. +REJECT_CASES = [ + ("a clean payload passes", '{"data":{"x":1}}', 0, ""), + ("an errors[]-only payload is refused", '{"errors":[{"message":"nope"}]}', + 1, "REFUSED-HERE"), + # The real trap: HTTP 200 carrying BOTH. A check that only looked for a + # missing `data` would call this a success. + ("data AND errors together is refused, not treated as partial success", + '{"data":{"updateProjectV2ItemFieldValue":null},"errors":[{"message":"x"}]}', + 1, "REFUSED-HERE"), + # Fail-closed arm. `jq -e 'has("errors")'` exits non-zero on BOTH "key + # absent" and "not JSON", so without its own arm this would read as clean. + ("an empty body is refused as unreadable, not accepted as error-free", + '', 1, "not readable JSON"), + ("a truncated/non-JSON body is refused as unreadable", + '{"data":{"upda', 1, "not readable JSON"), +] +for name, payload, want_rc, needle in REJECT_CASES: + rc, out = reject(payload) + record(rc == want_rc and (needle in out if needle else True), + f"graphql errors: {name}", + f"rc={rc} (want {want_rc}); looked for {needle!r} in: {out or ''}") + + +failed = [r for r in RESULTS if not r[0]] +print(f"\nbug-to-ready-selftest: {len(RESULTS) - len(failed)} passed, {len(failed)} failed") +sys.exit(1 if failed else 0)