feat(kanban): ask the board which columns are deploy states, not a list

.github/workflows/kanban-closure-router.yml

@@ -274,30 +274,55 @@ jobs:
           #
           # Only Done is guarded. A PR-derived Status is a deploy fact and may advance a
           # card normally.
-          # THE COLUMN LIST WAS STALE, AND THAT REOPENED THE HOLE IT CLOSES
-          # (backend#1846). `CURRENT_COL` is read from the BOARD, whose column is
-          # "Staging (agent review)" -- this list carried only the pre-rename
-          # "Staging (human review)". So an issue hand-closed while sitting in the
-          # agent-review column did not match, Done overwrote its deploy state,
-          # and kanban-archive.yml then hid the card entirely: exactly the
-          # sequence this guard exists to prevent, through a string that quietly
-          # stopped being true.
+          # WHICH COLUMNS ARE DEPLOY STATES, ASKED OF THE BOARD (backend#1846).
           #
-          # kanban-reconcile.yml's equivalent guard already lists both names. Two
-          # copies of one list, one updated at the rename and one not, is how this
-          # drifted -- so when the rename window closes, drop the legacy name in
-          # BOTH places.
+          # This was a hand-maintained list of six names, duplicated in
+          # kanban-reconcile.yml -- and it had already rotted once: it carried the
+          # pre-rename "Staging (human review)" and not the board's actual
+          # "Staging (agent review)", so a card hand-closed in that column lost its
+          # deploy state and kanban-archive.yml then hid it (.github#237). Fixing
+          # that instance left the CLASS: the next rename or inserted column
+          # reopens it, silently, in two files.
+          #
+          # The board already answers this. `$PROJ` carries the Status options in
+          # PIPELINE ORDER, so a deploy state is any column at or after "On dev"
+          # and at or before "Prod". An inserted column -- which is exactly how
+          # "Staging (agent review)" arrived -- is classified correctly with no
+          # edit here, and a renamed intermediate column keeps working because its
+          # POSITION is what matters, not its name.
+          #
+          # Two anchors instead of six names, and both are written by this same
+          # workflow, so .github#247's checker already asserts they exist.
+          col_index() {
+            echo "$PROJ" | jq -r --arg s "$1" \
+              '[.data.organization.projectV2.fields.nodes[]
+                | select(.name=="Status") | .options[].name] | index($s) // -1'
+          }
           if [ "$STATUS_NAME" = "Done" ]; then
-            case "${CURRENT_COL:-}" in
-              "On dev"|"Staging (agent review)"|"Staging (human review)"|"FR on staging"|"Ready for prod"|"Prod")
-                echo "::notice::#$NUMBER hand-closed but sits in '$CURRENT_COL', a deploy state - NOT setting Done (D8: follow the PR's stage)"
-                # AND SAY SO WHERE SOMEONE WILL SEE IT. Refusing is right, but it
-                # parks the card in a deploy state with no way to self-heal, and a
-                # run-log notice is invisible by the time anyone looks. The two
-                # real cases needed OPPOSITE answers -- backend#1493 had shipped
-                # via cli#452 and belonged in Prod; data-ingestors#488 was
-                # reverted and belonged in Done -- so no default is correct and
-                # only the person closing it knows which.
+            cur_i=$(col_index "${CURRENT_COL:-}")
+            dev_i=$(col_index "On dev")
+            prod_i=$(col_index "Prod")
+            # UNKNOWN MUST NOT FALL OPEN -- the other half of #1846. A column this
+            # workflow cannot place used to sail past the `case` and let Done erase
+            # a deploy state. Refusing costs a card sitting where it is; falling
+            # open erases the fact that it shipped.
+            if [ "$dev_i" -lt 0 ] || [ "$prod_i" -lt 0 ]; then
+              echo "::error::the board reports no 'On dev' or 'Prod' column, so a deploy state cannot be recognised - refusing to set Done on #$NUMBER"
+              exit 1
+            fi
+            if [ "$cur_i" -lt 0 ]; then
+              echo "::notice::#$NUMBER sits in '${CURRENT_COL:-<unreadable>}', which this board does not report as a column - NOT setting Done rather than guessing"
+              exit 0
+            fi
+            if [ "$cur_i" -ge "$dev_i" ] && [ "$cur_i" -le "$prod_i" ]; then
+              echo "::notice::#$NUMBER hand-closed but sits in '$CURRENT_COL', a deploy state - NOT setting Done (D8: follow the PR's stage)"
+              # AND SAY SO WHERE SOMEONE WILL SEE IT. Refusing is right, but it
+              # parks the card with no way to self-heal, and a run-log notice is
+              # invisible by the time anyone looks at the board. The two real
+              # cases needed OPPOSITE answers -- backend#1493 had shipped via
+              # cli#452 and belonged in Prod; data-ingestors#488 was reverted and
+              # belonged in Done -- so no default is correct and only the person
+              # closing it knows which.
                 CLOSE_NOTE="Closed while the board still shows \`$CURRENT_COL\`, which records a deployment."
                 CLOSE_NOTE="$CLOSE_NOTE The automation will not overwrite a deploy state with \`Done\` (RFC-BACKEND-1405 D8),"
                 CLOSE_NOTE="$CLOSE_NOTE so this card stays where it is until someone says which happened:"
@@ -306,8 +331,8 @@ jobs:
                 CLOSE_NOTE="$CLOSE_NOTE Both cases are real and they need opposite answers, which is why this is not decided automatically."
                 gh issue comment "$NUMBER" --repo "$REPO_FULL" --body "$CLOSE_NOTE" >/dev/null 2>&1 \
                   || echo "::warning::could not comment on #$NUMBER - it is parked in '$CURRENT_COL' with no note on the issue"
-                exit 0 ;;
-            esac
+              exit 0
+            fi
           fi
 
           # shellcheck disable=SC2016  # the $names here are GraphQL variables, not shell - keep literal

.github/workflows/kanban-deploy-state-selftest.yml

@@ -0,0 +1,44 @@
+name: Kanban deploy-state selftest
+
+# The classification that decides whether `Done` may overwrite a card's column
+# (backend#1846). It is asserted rather than trusted because its predecessor -- a
+# hand-maintained list of six column names -- rotted silently and cost a card its
+# deploy state (.github#237).
+#
+# The test reads `col_index()` OUT of kanban-closure-router.yml rather than
+# copying it, so it cannot go green against a stale duplicate. Offline, no token.
+
+on:
+  pull_request:
+    paths:
+      - .github/workflows/kanban-closure-router.yml
+      - .github/workflows/kanban-deploy-state-selftest.yml
+      - scripts/tests/kanban-deploy-state-selftest.py
+  push:
+    branches: [main, develop, staging]
+    paths:
+      - .github/workflows/kanban-closure-router.yml
+      - .github/workflows/kanban-deploy-state-selftest.yml
+      - scripts/tests/kanban-deploy-state-selftest.py
+
+permissions:
+  contents: read
+
+concurrency:
+  group: kanban-deploy-state-selftest-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  selftest:
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+      - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0
+        with:
+          python-version: '3.12'
+      # PyYAML only: the test parses the workflow to extract the function under
+      # test. `pip install` without setup-python is what left the bricked-PR
+      # audit unable to start on Ubuntu 24.04's PEP 668 python (.github#244).
+      - run: pip install --quiet pyyaml
+      - run: python scripts/tests/kanban-deploy-state-selftest.py

scripts/tests/kanban-deploy-state-selftest.py

@@ -0,0 +1,148 @@
+#!/usr/bin/env python3
+"""The deploy-state classification is read OUT of the workflow and exercised.
+
+WHY THIS EXISTS (backend#1846)
+
+`kanban-closure-router.yml` must never overwrite a deploy state with `Done`
+(RFC-BACKEND-1405 D8). It decided that from a hand-maintained list of six column
+names, duplicated in `kanban-reconcile.yml` -- and the list rotted: it carried
+the pre-rename "Staging (human review)" while the board's column is
+"Staging (agent review)", so a card hand-closed there lost its deploy state and
+`kanban-archive.yml` hid it (.github#237).
+
+The classification now comes from the board's own option ORDER: a deploy state is
+any column at or after `On dev` and at or before `Prod`. That is what makes an
+inserted column -- which is how "Staging (agent review)" arrived -- correct with
+no edit.
+
+WHY IT IS EXTRACTED RATHER THAN COPIED
+
+A copy of the logic here would let the workflow drift while this file stays
+green, which is the same defect class the classification itself had. So the
+`col_index` function is pulled from the YAML and sourced. If someone renames or
+reshapes it, this test stops finding it and fails loudly rather than testing a
+stale duplicate.
+
+Exit 0 when every case behaves as specified.
+"""
+from __future__ import annotations
+
+import json
+import os
+import re
+import subprocess
+import sys
+
+import yaml
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+WF = os.path.join(HERE, os.pardir, os.pardir,
+                  ".github", "workflows", "kanban-closure-router.yml")
+
+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 extract_col_index() -> str:
+    """Pull `col_index()` out of the workflow's run: block, verbatim."""
+    doc = yaml.safe_load(open(WF))
+    runs = [s["run"] for j in doc["jobs"].values()
+            for s in j.get("steps", []) if "run" in s]
+    for body in runs:
+        m = re.search(r"^\s*col_index\(\) \{.*?^\s*\}\s*$", body,
+                      re.S | re.M)
+        if m:
+            # Strip the workflow's indentation so it parses standalone.
+            block = m.group(0)
+            indent = len(block) - len(block.lstrip())
+            return "\n".join(ln[indent:] if ln[:indent].isspace() else ln
+                             for ln in block.splitlines())
+    sys.exit("could not find col_index() in the workflow — did it get renamed? "
+             "This test refuses to fall back to a copy.")
+
+
+COL_INDEX = extract_col_index()
+
+# The live board's order, as the API returns it.
+BOARD = ["Backlog", "North Stars", "Ready", "In progress", "Code review",
+         "On dev", "Staging (agent review)", "FR on staging", "Ready for prod",
+         "Prod", "Done", "Cancelled"]
+
+
+def proj(names) -> str:
+    return json.dumps({"data": {"organization": {"projectV2": {"fields": {
+        "nodes": [{"name": "Status",
+                   "options": [{"name": n} for n in names]}]}}}}})
+
+
+def classify(current: str, names=None) -> str:
+    """Run the EXTRACTED function plus the guard's own comparison."""
+    names = BOARD if names is None else names
+    script = f"""
+set -euo pipefail
+PROJ='{proj(names)}'
+{COL_INDEX}
+cur_i=$(col_index "{current}")
+dev_i=$(col_index "On dev")
+prod_i=$(col_index "Prod")
+if [ "$dev_i" -lt 0 ] || [ "$prod_i" -lt 0 ]; then echo ANCHORS_MISSING; exit 0; fi
+if [ "$cur_i" -lt 0 ]; then echo UNKNOWN; exit 0; fi
+if [ "$cur_i" -ge "$dev_i" ] && [ "$cur_i" -le "$prod_i" ]; then
+  echo DEPLOY_STATE
+else
+  echo FREE
+fi
+"""
+    out = subprocess.run(["bash", "-c", script], capture_output=True, text=True)
+    if out.returncode != 0:
+        return f"ERROR: {out.stderr.strip()}"
+    return out.stdout.strip()
+
+
+# 1. Every deploy state, including the one the old list missed.
+for col in ("On dev", "Staging (agent review)", "FR on staging",
+            "Ready for prod", "Prod"):
+    record(classify(col) == "DEPLOY_STATE",
+           f"{col!r} is a deploy state", f"-> {classify(col)}")
+
+# 2. Everything before On dev is free to be overwritten by Done.
+for col in ("Backlog", "Ready", "In progress", "Code review"):
+    record(classify(col) == "FREE",
+           f"{col!r} is not a deploy state", f"-> {classify(col)}")
+
+# 3. TERMINAL columns sit AFTER Prod in the order, so the `<= Prod` bound is
+#    what stops them being treated as deploy states. Without it a card already
+#    in Done would refuse to be set to Done -- harmless, but it would also make
+#    Cancelled unreachable.
+for col in ("Done", "Cancelled"):
+    record(classify(col) == "FREE",
+           f"{col!r} is terminal, not a deploy state", f"-> {classify(col)}")
+
+# 4. THE CASE THE OLD LIST GOT WRONG. A column inserted between the anchors is
+#    classified correctly with no edit here -- which is exactly how
+#    "Staging (agent review)" arrived and why the list rotted.
+inserted = BOARD[:7] + ["Staging (robot review)"] + BOARD[7:]
+record(classify("Staging (robot review)", inserted) == "DEPLOY_STATE",
+       "a NEWLY inserted column between the anchors is a deploy state",
+       "no edit to the workflow required — this is the rot the list had")
+
+# 5. UNKNOWN MUST NOT FALL OPEN.
+record(classify("Some Column Nobody Declared") == "UNKNOWN",
+       "a column the board does not report is UNKNOWN, not free",
+       "the old `case` fell through and let Done erase the deploy state")
+
+record(classify("") == "UNKNOWN",
+       "an unreadable current column is UNKNOWN, not free", "empty CURRENT_COL")
+
+# 6. A board missing an anchor cannot be classified at all.
+record(classify("On dev", [n for n in BOARD if n != "Prod"]) == "ANCHORS_MISSING",
+       "a board with no 'Prod' column refuses rather than guessing",
+       "the guard exits 1 on this, which is the fail-closed half")
+
+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)