Wave 3: predecessor references with a release gate, and critical-constraint reopen alerts
Predecessors are real references now - data.predecessors holds work-package ids, replacing a free-text SOP phase label that couldn't express "WP04 waits on WP02" and gated nothing. The SOP phase survives beside it as the descriptive "Sequence phase" field. - readiness() has two gates: constraints clear AND every predecessor Closed. The banner, sticky bar, left rail, dashboard Gates column and the ready counters all reflect the second one. - Enforced server-side by enforce_release_gates() on every path that sets a status — the plain upsert included, since that's how the browser and the offline outbox save. /issue and /status would otherwise have been ways around it. - Cycles are refused directly and through a chain, with a message naming the package that already waits on this one. The Creator's picker also hides itself and its own descendants, so a cycle is hard to build in the first place. - A deleted predecessor does not block: it would freeze everything downstream of a package someone removed. - The gate is refusable, on purpose. Planners release ahead of upstream close-out, so an explicit reason (data.gateOverride) allows it, gets a gate_overridden audit event naming what was skipped, and prints on the package. A blank reason is not an override, and changing the predecessor set clears it. The dashboard won't release a blocked package at all — it points at the form where the reason is captured. Critical constraints reopened after release - Reopening a SOP-critical constraint on a released package emails the owner, PM, CM and the package's distribution list (minus whoever did it) and writes a constraint_reopened audit event. - Detected by diffing the incoming constraints against the stored ones inside the normal upsert rather than via a new endpoint: the sync outbox only replays POST /api/wps, so a dedicated route would be lost offline. It fires only on a real cleared→open transition, so re-saving an already-open constraint doesn't re-announce, and never before release or for a non-critical constraint. - Bodies carry the constraint name, WP number and a link — never package contents. Verified: 139 API checks on one fresh database (44 permissions + 22 password reset + 34 search/localization + 39 gates/notifications), including every bypass path, cycle shapes, the deleted-predecessor case, blank-reason overrides, and the four recipients confirmed both in the outbox and on the wire against a local SMTP sink. 27 driven UI checks against the real Creator page in headless Chrome covering the picker, the override prompt (accept and cancel), override invalidation, the cycle exclusions and the dashboard refusal. Screenshots reviewed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
217
server/app.py
217
server/app.py
@@ -873,6 +873,183 @@ def delete_sop(sop_id: str, user: models.User = Depends(auth.get_current_user),
|
||||
return {"deleted": sop_id}
|
||||
|
||||
|
||||
# ── Release gates (constraints + predecessors) ─────────────────────────────────
|
||||
# Status ladder, mirrored in the front end (wp-creation-app.js STATUS_ORDER).
|
||||
STATUS_ORDER = ["Draft", "Scheduled", "Issued", "In Progress", "Issue", "QC", "Closed"]
|
||||
ISSUED_IDX = STATUS_ORDER.index("Issued")
|
||||
DONE_STATUS = "Closed"
|
||||
|
||||
|
||||
def _released(status: Optional[str]) -> bool:
|
||||
"""Has this package been released to the field (Issued or anything after)?"""
|
||||
try:
|
||||
return STATUS_ORDER.index(status or "") >= ISSUED_IDX
|
||||
except ValueError:
|
||||
return False
|
||||
|
||||
|
||||
def predecessor_ids(data: Optional[dict]) -> list[str]:
|
||||
"""Work-package ids this package waits on. Ignores anything malformed rather
|
||||
than failing a save — a bad id simply can't block."""
|
||||
raw = (data or {}).get("predecessors") or []
|
||||
if not isinstance(raw, list):
|
||||
return []
|
||||
return [p for p in raw if isinstance(p, str) and _ID_RE.match(p)]
|
||||
|
||||
|
||||
def gate_override(data: Optional[dict]) -> Optional[dict]:
|
||||
"""A deliberate, reasoned override of the predecessor gate. Planners legitimately
|
||||
need to stage packages ahead of the work finishing, so the gate is refusable —
|
||||
but only explicitly, and it lands in the audit log."""
|
||||
ov = (data or {}).get("gateOverride")
|
||||
if isinstance(ov, dict) and str(ov.get("reason") or "").strip():
|
||||
return ov
|
||||
return None
|
||||
|
||||
|
||||
def blocking_predecessors(db: Session, wp_id: Optional[str], data: Optional[dict]) -> list[dict]:
|
||||
"""Predecessors that are not Closed yet. A predecessor that no longer exists is
|
||||
NOT blocking — a deleted package must not freeze everything downstream."""
|
||||
ids = [p for p in predecessor_ids(data) if p != wp_id]
|
||||
if not ids:
|
||||
return []
|
||||
rows = db.scalars(select(models.WorkPackage).where(models.WorkPackage.id.in_(ids))).all()
|
||||
return [
|
||||
{"id": r.id, "number": r.number, "subject": r.subject, "status": r.status}
|
||||
for r in rows if r.status != DONE_STATUS
|
||||
]
|
||||
|
||||
|
||||
def check_predecessor_cycle(db: Session, wp_id: str, data: Optional[dict]) -> None:
|
||||
"""Refuse a predecessor set that would make A wait on itself (directly or
|
||||
through a chain). Walks the graph from the proposed predecessors; the visited
|
||||
set also bounds the walk, so a cycle that already exists elsewhere in the data
|
||||
can't spin here."""
|
||||
start = [p for p in predecessor_ids(data)]
|
||||
if wp_id in start:
|
||||
raise HTTPException(status_code=400, detail="A work package cannot be its own predecessor.")
|
||||
seen, stack = set(), list(start)
|
||||
while stack:
|
||||
cur = stack.pop()
|
||||
if cur in seen:
|
||||
continue
|
||||
seen.add(cur)
|
||||
row = db.get(models.WorkPackage, cur)
|
||||
if row is None:
|
||||
continue
|
||||
nxt = predecessor_ids(row.data)
|
||||
if wp_id in nxt:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail=f"That would create a circular dependency ({row.number or row.id} already waits on this package).",
|
||||
)
|
||||
stack.extend(n for n in nxt if n not in seen)
|
||||
|
||||
|
||||
def enforce_release_gates(db: Session, wp_id: Optional[str], data: Optional[dict],
|
||||
new_status: str, old_status: Optional[str]) -> None:
|
||||
"""Refuse a move to Issued (or beyond) while a release gate is unmet. Applied to
|
||||
every path that can set a status — the plain upsert included, since that's how
|
||||
the browser and the offline outbox save."""
|
||||
if not _released(new_status) or _released(old_status):
|
||||
return # not a release transition
|
||||
constraints = (data or {}).get("constraints") or []
|
||||
open_names = [c.get("name") for c in constraints if isinstance(c, dict) and c.get("status") == "open"]
|
||||
if open_names:
|
||||
raise HTTPException(status_code=409, detail={
|
||||
"message": "Open constraints block release", "open": open_names,
|
||||
})
|
||||
blockers = blocking_predecessors(db, wp_id, data)
|
||||
if blockers and not gate_override(data):
|
||||
raise HTTPException(status_code=409, detail={
|
||||
"message": "Predecessors are not closed yet",
|
||||
"blocking": [f"{b['number'] or b['id']} ({b['status']})" for b in blockers],
|
||||
})
|
||||
|
||||
|
||||
# ── Critical-constraint notification ───────────────────────────────────────────
|
||||
# A constraint flagged CRITICAL on the SOP, reopened after the package was
|
||||
# released, is announced by email. We detect it by comparing the incoming
|
||||
# constraints against the stored ones during the normal upsert rather than adding a
|
||||
# separate endpoint: the browser saves through the sync outbox, which only replays
|
||||
# POST /api/wps, so anything hung off another route would be lost offline.
|
||||
def reopened_critical(old_data: Optional[dict], new_data: Optional[dict]) -> list[str]:
|
||||
old = {}
|
||||
for c in (old_data or {}).get("constraints") or []:
|
||||
if isinstance(c, dict) and c.get("name"):
|
||||
old[c["name"]] = c.get("status")
|
||||
out = []
|
||||
for c in (new_data or {}).get("constraints") or []:
|
||||
if not isinstance(c, dict) or not c.get("critical"):
|
||||
continue
|
||||
name = c.get("name")
|
||||
if not name or c.get("status") != "open":
|
||||
continue
|
||||
if old.get(name) not in (None, "open"): # was cleared/na, now open
|
||||
out.append(name)
|
||||
return out
|
||||
|
||||
|
||||
def project_sop_team(db: Session, project_id: Optional[str]) -> list[str]:
|
||||
"""PM + CM user ids from the project's most recent complete SOP."""
|
||||
if not project_id:
|
||||
return []
|
||||
sop = db.scalars(
|
||||
select(models.Sop)
|
||||
.where((models.Sop.project_id == project_id) & (models.Sop.complete.is_(True)))
|
||||
.order_by(models.Sop.updated_at.desc())
|
||||
.limit(1)
|
||||
).first()
|
||||
if not sop:
|
||||
return []
|
||||
proj = (sop.data or {}).get("project") or {}
|
||||
return [i for i in (proj.get("pmId"), proj.get("cmId")) if i]
|
||||
|
||||
|
||||
def hold_body(user: "models.User", wp: "models.WorkPackage", names: list[str],
|
||||
actor: "models.User", link: str) -> str:
|
||||
# Constraint names and a WP number only — no package contents, same rule as the
|
||||
# assignment mail.
|
||||
who = user.full_name or user.username
|
||||
by = actor.full_name or actor.username
|
||||
which = ", ".join(names)
|
||||
return (
|
||||
f"Hi {who},\n\n"
|
||||
f"A critical constraint was reopened on {wp.number or 'a work package'} "
|
||||
f"after it was released to the field, so the package is on hold.\n\n"
|
||||
f"Constraint: {which}\n"
|
||||
f"Reopened by: {by}\n\n"
|
||||
f"Open the package:\n{link}\n"
|
||||
)
|
||||
|
||||
|
||||
def notify_critical_reopen(db: Session, wp: "models.WorkPackage", names: list[str],
|
||||
actor: "models.User") -> list["models.Notification"]:
|
||||
"""Owner + PM + CM + everyone on the package's distribution list, minus whoever
|
||||
did it (they already know) and minus duplicates."""
|
||||
ids = []
|
||||
if wp.assignee_id:
|
||||
ids.append(wp.assignee_id)
|
||||
ids.extend(project_sop_team(db, wp.project_id))
|
||||
ids.extend([i for i in ((wp.data or {}).get("distributionIds") or []) if isinstance(i, str)])
|
||||
seen, out = set(), []
|
||||
link = wp_link(db, wp)
|
||||
for uid in ids:
|
||||
if uid in seen or uid == actor.id:
|
||||
continue
|
||||
seen.add(uid)
|
||||
u = db.get(models.User, uid)
|
||||
if not u or not u.is_active:
|
||||
continue
|
||||
out.append(notify.enqueue(
|
||||
db, user=u, kind="wp_constraint_reopened",
|
||||
subject=f"On hold: {wp.number or 'work package'} — critical constraint reopened",
|
||||
body=hold_body(u, wp, names, actor, link),
|
||||
link=link, wp_id=wp.id, project_id=wp.project_id,
|
||||
))
|
||||
return out
|
||||
|
||||
|
||||
# ── Work Packages ────────────────────────────────────────────────────────────
|
||||
@app.post("/api/wps")
|
||||
def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)):
|
||||
@@ -886,9 +1063,15 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User =
|
||||
is_new = wp is None
|
||||
old_status = None if is_new else wp.status
|
||||
old_assignee = None if is_new else wp.assignee_id
|
||||
old_data = None if is_new else (wp.data or {})
|
||||
if wp is None:
|
||||
wp = models.WorkPackage(id=body.id or gen_id("wp"))
|
||||
db.add(wp)
|
||||
# Predecessors: reject a cycle, and refuse a release while a gate is unmet.
|
||||
# Both run before anything is written so a rejected save changes nothing.
|
||||
wp_id_for_checks = body.id or wp.id
|
||||
check_predecessor_cycle(db, wp_id_for_checks, body.data)
|
||||
enforce_release_gates(db, wp_id_for_checks, body.data, body.status, old_status)
|
||||
wp.project_id = body.project_id
|
||||
wp.sop_id = body.sop_id
|
||||
wp.parent_id = body.parent_id
|
||||
@@ -910,6 +1093,25 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User =
|
||||
_act, _detail = "updated", {"status": wp.status}
|
||||
log_event(db, user, _act, "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id), detail=_detail)
|
||||
notifs = []
|
||||
# A reasoned override of the predecessor gate is worth its own audit line —
|
||||
# "released early, and here's why" is exactly what a reviewer looks for later.
|
||||
ov = gate_override(body.data)
|
||||
if ov and _released(wp.status) and not _released(old_status):
|
||||
blockers = blocking_predecessors(db, wp.id, body.data)
|
||||
log_event(db, user, "gate_overridden", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id),
|
||||
detail={"reason": str(ov.get("reason"))[:300],
|
||||
"blocking": [b["number"] or b["id"] for b in blockers]})
|
||||
# A critical constraint reopened after release: log it and tell the people who
|
||||
# need to know (owner, PM, CM, distribution).
|
||||
if not is_new and _released(old_status):
|
||||
reopened = reopened_critical(old_data, wp.data)
|
||||
if reopened:
|
||||
log_event(db, user, "constraint_reopened", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id),
|
||||
detail={"critical": reopened, "status": wp.status})
|
||||
notifs.extend(notify_critical_reopen(db, wp, reopened, user))
|
||||
# Notify a newly-assigned owner (skip self-assignment).
|
||||
notif = None
|
||||
if new_assignee and new_assignee != old_assignee and new_assignee != user.id:
|
||||
@@ -927,7 +1129,9 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User =
|
||||
db.commit()
|
||||
db.refresh(wp)
|
||||
if notif is not None:
|
||||
background_tasks.add_task(notify.deliver, notif.id)
|
||||
notifs.append(notif)
|
||||
for n in notifs:
|
||||
background_tasks.add_task(notify.deliver, n.id)
|
||||
return wp.to_dict()
|
||||
|
||||
|
||||
@@ -1048,16 +1252,13 @@ def delete_wp(wp_id: str, user: models.User = Depends(auth.get_current_user), db
|
||||
|
||||
@app.post("/api/wps/{wp_id}/issue")
|
||||
def issue_wp(wp_id: str, user: models.User = Depends(auth.get_current_user), db: Session = Depends(get_db)):
|
||||
"""Release a Work Package to the field. Refuses if any constraint is still
|
||||
open (the AWP release gate)."""
|
||||
"""Release a Work Package to the field. Refuses while a release gate is unmet:
|
||||
any open constraint, or a predecessor package that isn't Closed."""
|
||||
wp = db.get(models.WorkPackage, wp_id)
|
||||
if not wp:
|
||||
raise HTTPException(status_code=404, detail="Work Package not found")
|
||||
require_project_access(db, user, wp.project_id)
|
||||
constraints = (wp.data or {}).get("constraints") or []
|
||||
open_names = [c.get("name") for c in constraints if c.get("status") == "open"]
|
||||
if open_names:
|
||||
raise HTTPException(status_code=409, detail={"message": "Open constraints block issuance", "open": open_names})
|
||||
enforce_release_gates(db, wp.id, wp.data, "Issued", wp.status)
|
||||
wp.status = "Issued"
|
||||
wp.issued_at = models.utcnow()
|
||||
log_event(db, user, "issued", "wp", wp.id, project_id=wp.project_id,
|
||||
@@ -1074,6 +1275,8 @@ def set_wp_status(wp_id: str, body: StatusIn, user: models.User = Depends(auth.g
|
||||
raise HTTPException(status_code=404, detail="Work Package not found")
|
||||
require_project_access(db, user, wp.project_id)
|
||||
old_status = wp.status
|
||||
# Same gates as /issue — this route must not be a way around them.
|
||||
enforce_release_gates(db, wp.id, wp.data, body.status, old_status)
|
||||
wp.status = body.status
|
||||
if body.status == "Issued" and wp.issued_at is None:
|
||||
wp.issued_at = models.utcnow()
|
||||
|
||||
Reference in New Issue
Block a user