From 24f60151e5e4c7ee1101706364a4e94ba8cff1ce Mon Sep 17 00:00:00 2001 From: "n.siegfried" Date: Thu, 20 Aug 2026 18:17:31 -0700 Subject: [PATCH] BL-021 - the critical-reopen mail reaches the PM and CM, at last project_sop_team() read sop.data['project']; pushSOP stores every row as data={sop, state}, so the project block is one level deeper. The lookup returned [] for every real row, silently, and the on-hold email promised to 'Owner + PM + CM + distribution' has reached only owner + distribution since the day it shipped. One line: the same nested-first tolerant read project_qa_group has used all along (whose docstring logged this very bug). New probe critical_reopen_check (11): the fixture writes the PRODUCTION shape - a hand-built flat row would have passed against the bug, which is exactly how it went unverified this long. Sink-verified end to end: assignee + PM + CM and nobody else; constraint name, title, location, deep link and the house footer in the body (the footer this body alone used to lack, fixed at CR-014). Items: BL-021 (closed), CR-011 recipients. Co-Authored-By: Claude Fable 5 --- docs/reference/file-map.md | 1 + docs/waves/backlog.md | 2 +- server/app.py | 13 +++- tests/critical_reopen_check.py | 135 +++++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 tests/critical_reopen_check.py diff --git a/docs/reference/file-map.md b/docs/reference/file-map.md index 7c9af22..02597e9 100644 --- a/docs/reference/file-map.md +++ b/docs/reference/file-map.md @@ -312,6 +312,7 @@ The August 20 integration adds: ```bash python tests/assets_check.py # D11 - Micron picker: read-only, degrades 31 checks +python tests/critical_reopen_check.py # BL-021 - on-hold mail reaches PM + CM 11 checks ``` **Three probes were re-pointed at `T7.1`.** `sections_check.py` 5b drove the live diff --git a/docs/waves/backlog.md b/docs/waves/backlog.md index 34c6d44..ffc7e6f 100644 --- a/docs/waves/backlog.md +++ b/docs/waves/backlog.md @@ -474,7 +474,7 @@ deliberately deferred. kept, `T7.2`'s side navigation is the place to make saving obvious enough that the prompt stops being a surprise. -### BL-021 — `project_sop_team()` reads a path `pushSOP` never writes +### BL-021 — CLOSED 2026-08-20 (`project_sop_team()` reads nested-first; `critical_reopen_check` 11, sink-verified) - **Found during:** T7.6 - **Where:** `server/app.py`, `project_sop_team()` diff --git a/server/app.py b/server/app.py index 55099e8..1a381fe 100644 --- a/server/app.py +++ b/server/app.py @@ -1623,15 +1623,22 @@ def project_sop_team(db: Session, project_id: Optional[str]) -> list[str]: ).first() if not sop: return [] - proj = (sop.data or {}).get("project") or {} + data = sop.data or {} + # BL-021, fixed 2026-08-20: pushSOP writes the row as data={sop, state}, so + # the project block lives at data['sop']['project']. This read was one + # level too shallow - always {} - and the critical-reopen mail never + # reached the PM or CM its docstring promises. Same tolerant read as + # project_qa_group below: nested shape first, flat shape for hand-written rows. + proj = ((data.get("sop") or {}).get("project") + or data.get("project") or {}) return [i for i in (proj.get("pmId"), proj.get("cmId")) if i] def project_qa_group(db: Session, project_id: Optional[str]) -> list["models.User"]: """D2: the QA group named on the project's latest complete SOP. pushSOP writes the row as data={sop, state}, so the project block is data['sop']['project'] - - note that project_sop_team above reads data['project'], which that shape never - has (BL-021, logged, not fixed here).""" + project_sop_team above read the flat shape until BL-021 was fixed + (2026-08-20); both now read nested-first, exactly alike.""" if not project_id: return [] sop = db.scalars( diff --git a/tests/critical_reopen_check.py b/tests/critical_reopen_check.py new file mode 100644 index 0000000..1a3634a --- /dev/null +++ b/tests/critical_reopen_check.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Does the critical-reopen mail reach the PM and CM? — BL-021, fixed 2026-08-20. + +`project_sop_team()` read `sop.data['project']`, but `pushSOP` stores every row +as `data={sop, state}` — the project block is one level deeper. The lookup +returned `[]` for every real row, so the on-hold email's recipient list was +silently reduced to assignee + distribution: the PM and CM named in +`notify_critical_reopen`'s own docstring never got it, from the day it shipped. + +The fixture writes the PRODUCTION shape (nested under 'sop'), because a +hand-built flat row would have passed against the bug — which is exactly how it +went unverified this long. Sink pattern from qa_gate_check, one implementation. + +Boots its own throwaway SQLite + uvicorn; run it alone, not back to back. +Exit 0 all passed, 1 a failure, 2 could not run. +""" +import io +import json +import os +import re +import sys +import tempfile + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +import cdp # noqa: E402 +from browser_check import seed, start_server, chk, _PASS, _FAIL # noqa: E402 +from sections_check import set_sop # noqa: E402 +from qa_gate_check import SmtpSink, api, wait_for # noqa: E402 + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def ascii_(v, n=280): + return re.sub(r"\s+", " ", str(v)).encode("ascii", "replace").decode()[:n] + + +def set_team(pm_id, cm_id): + """The PRODUCTION shape: data['sop']['project'], as pushSOP writes it.""" + from server.db import SessionLocal + from server import models + with SessionLocal() as db: + sop = db.get(models.Sop, "sopA") + data = json.loads(json.dumps(sop.data or {})) + proj = data.setdefault("sop", {}).setdefault("project", {}) + proj["pmId"], proj["cmId"] = pm_id, cm_id + sop.data = data + db.commit() + + +def main(): + print("\n1. the read matches the written shape (static)") + src = io.open(os.path.join(ROOT, "server", "app.py"), encoding="utf-8").read() + fn = src[src.index("def project_sop_team"):src.index("def project_qa_group")] + chk("project_sop_team reads the nested data['sop']['project'] first", + '.get("sop")' in fn and '.get("project")' in fn) + + tmpdir = tempfile.mkdtemp(prefix="wpsuite-reopen-") + db_path = os.path.join(tmpdir, "check.db") + server = None + sink = SmtpSink() + sink.start() + try: + tok = seed(db_path) + set_sop(db_path, {}) + set_team("user_sue", "user_pat") + port = cdp.free_port() + base = "http://127.0.0.1:%d" % port + server = start_server(port, db_path) + root = tok["root"] + + api(base, "/api/settings", root, "PUT", { + "email_enabled": True, "smtp_host": "127.0.0.1", "smtp_port": sink.port, + "smtp_use_tls": False, "from_addr": "suite@sink.local", + "app_base_url": base}) + + print("\n2. a released package, its critical constraint reopened") + body = { + "id": "wpCR1", "project_id": "projA", "number": "CR-01", + "subject": "energize MCC-4", "status": "In Progress", + "assignee_id": "user_mix", + "data": {"location": "B-100 / Level 2", + "constraints": [{"name": "Power shutdown", "status": "cleared", + "critical": True, "comment": ""}]}} + code, _ = api(base, "/api/wps", root, "POST", body) + chk("the package saves", code == 200, code) + # The creation enqueues a wp_assigned mail delivered by a background + # task; wait for it BEFORE clearing or it leaks into the reopen count. + wait_for(lambda: len(sink.messages) >= 1, 10) + code, _ = api(base, "/api/wps/wpCR1/status", root, "POST", {"status": "Issued"}) + chk("...and releases (the critical constraint is cleared)", code == 200, code) + sink.messages.clear() + + body["status"] = "Issued" + body["data"]["constraints"][0]["status"] = "open" + code, wp = api(base, "/api/wps", root, "POST", body) + chk("reopening the critical constraint saves through the normal upsert", + code == 200, code) + + print("\n3. the mail reaches everyone the docstring promises") + chk("three messages: assignee + PM + CM (the actor is excluded)", + wait_for(lambda: len(sink.messages) == 3, 15), len(sink.messages)) + rcpts = sorted(m["to"][0] for m in sink.messages) + chk("...the PM and CM are among them - THE BL-021 fix, sink-verified", + "sue@example.test" in rcpts and "pat@example.test" in rcpts, ascii_(rcpts)) + chk("...and the assignee, exactly them, nobody twice", + rcpts == ["mix@example.test", "pat@example.test", "sue@example.test"], + ascii_(rcpts)) + text = next((m["text"] for m in sink.messages if "On hold:" in m["data"]), "") + chk("the body names the constraint and the package, title included (CR-014)", + "Power shutdown" in text and "CR-01" in text and "energize MCC-4" in text, + ascii_(text, 300)) + chk("...and where the work happens, and a deep link to THAT package", + "B-100 / Level 2" in text + and "/wp-creation-index.html?project=projA&wp=wpCR1" in text, ascii_(text, 300)) + chk("...in the house convention (the footer this body alone used to lack)", + "automated message from the Work Package Suite" in text) + _, ev = api(base, "/api/audit?entity_type=wp&entity_id=wpCR1" + "&action=constraint_reopened", root) + chk("the reopen is in the audit history", bool(ev), ascii_(ev[:1] if ev else ev)) + finally: + sink.stop() + if server: + server.terminate() + + print("\n" + "-" * 54) + print("%d/%d checks passed." % (len(_PASS), len(_PASS) + len(_FAIL))) + for f in _FAIL: + print(" - " + f) + return 1 if _FAIL else 0 + + +if __name__ == "__main__": + sys.exit(main())