T7.3 - CR-015/A1/D4: the hold clears when the constraints do
ROOT CAUSE, exactly (the done-when asks for it):
Hold state was stored, twice, and derived nowhere.
1) Client: submitHold() wrote prevStatus='Issue', destroying the status the
hold interrupted at the moment it was placed - there was never anything to
return to. Clearing the last constraint then fell into the "Mark it as
Issued now?" confirm, because STATUS_ORDER.indexOf('Issue') is -1 and -1
reads as "before Issued". Decline it and the package stayed on hold with
zero open constraints, forever - the exact state reproduced live in front
of the Micron team.
2) Server: server/app.py's STATUS_ORDER put "Issue" at index 4, so
_released('Issue') was true and every transition OUT of hold skipped
enforce_release_gates() as "already released". POST /api/wps/{id}/status
could walk a held package to Issued past its open constraint. The comment
claimed the ladder was "mirrored in the front end"; the front end's ladder
has no 'Issue' in it at all.
What changed:
- setConstraint() recalculates hold state on EVERY constraint change: clearing
the last open constraint on a held package releases it immediately - no
refresh, no dialog - back to the status recorded on the hold entry (`from`),
which now rides on data.holds and survives save/reload.
- Every hold and release is history: pkgHolds entries carry ts, by, from/to,
reason; the exported Hold Log gained a By column; the server writes
hold_logged / hold_released audit rows (with the reason from data.holds) on
both the upsert and the /status endpoint.
- _released() no longer counts the hold: 'Issue' is a branch, not a rung.
Leaving hold to a field state re-runs the gates; entering hold never did and
still does not. The critical-reopen email keeps its old reach ("has been in
the field" includes on-hold).
- A1 preserved by name and by test: confirmEarlyRelease() still the one place
a gate override is written (comment-stripped grep asserts exactly one
pkgGateOverride assignment), still reason-first, still logged server-side.
D4 - what Urgent does (amended Aug 18): surface the audited path, add no new
one. confirmEarlyRelease() now also covers open constraints, but only for an
Urgent package, and the override must NAME every constraint it crosses - the
server refuses coverage by an old reason. The release banner gives an Urgent
package the override as its primary action (a real <button>); Normal and High
see nothing new and keep the same hard refusal, asserted per priority.
Banner button styled from tokens only; the banner now wraps at narrow widths.
Product question raised, not decided (per CLAUDE.md "asking versus assuming"):
Issue (hold) remains selectable from Draft and Scheduled, as it was before.
The done-when names no state list, so nothing was restricted. If a pre-release
hold is meaningless, closing it off is a one-line follow-up - needs Nick.
Verification (each probe run alone): NEW tests/hold_check.py 50/50, including
the clear-last-constraint regression specifically, the D4 priority matrix
against the server (six 409/200 cases), hold_logged/hold_released audit rows,
and an AST sweep proving every wp.status assignment in server/app.py sits in
a function that runs enforce_release_gates. Regressions: frame_check 39/39,
aggregates_check 16/16.
Items: CR-015, A1, D4 (X2 correction already recorded Aug 18)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1405,14 +1405,23 @@ def delete_sop(sop_id: str, user: models.User = Depends(auth.get_current_user),
|
||||
|
||||
|
||||
# ── Release gates (constraints + predecessors) ─────────────────────────────────
|
||||
# Status ladder, mirrored in the front end (wp-creation-app.js STATUS_ORDER).
|
||||
# Status ladder. The front end's STATUS_ORDER (wp-creation-app.js) has no "Issue"
|
||||
# in it at all - the hold is a BRANCH off the released states, not a rung. It sits
|
||||
# in this list so unknown statuses can still be told apart from known ones, but
|
||||
# _released() must never count it: counting it is what let every transition out of
|
||||
# hold skip the release gates as "already released", which made /status a side
|
||||
# door past an open constraint (CR-015 / T7.3).
|
||||
STATUS_ORDER = ["Draft", "Scheduled", "Issued", "In Progress", "Issue", "QC", "Closed"]
|
||||
ISSUED_IDX = STATUS_ORDER.index("Issued")
|
||||
DONE_STATUS = "Closed"
|
||||
HOLD_STATUS = "Issue"
|
||||
|
||||
|
||||
def _released(status: Optional[str]) -> bool:
|
||||
"""Has this package been released to the field (Issued or anything after)?"""
|
||||
"""Is this status a field state (Issued or beyond)? The hold is NOT one: a
|
||||
package leaves hold through the gates, and enters it without them."""
|
||||
if status == HOLD_STATUS:
|
||||
return False
|
||||
try:
|
||||
return STATUS_ORDER.index(status or "") >= ISSUED_IDX
|
||||
except ValueError:
|
||||
@@ -1438,6 +1447,23 @@ def gate_override(data: Optional[dict]) -> Optional[dict]:
|
||||
return None
|
||||
|
||||
|
||||
def _urgent_constraint_override(data: Optional[dict], open_names: list) -> bool:
|
||||
"""D4: only an URGENT package may release past an open constraint, and only
|
||||
through the audited override - the same gateOverride record the predecessor
|
||||
gate uses, never a separate path. The override must NAME every constraint it
|
||||
covers: a constraint opened after the reason was written cannot ride through
|
||||
on it. Normal and High are unchanged - a hard refusal."""
|
||||
if str((data or {}).get("priority") or "") != "Urgent":
|
||||
return False
|
||||
ov = gate_override(data)
|
||||
if not ov:
|
||||
return False
|
||||
covered = ov.get("constraints")
|
||||
if not isinstance(covered, list):
|
||||
return False
|
||||
return {str(n) for n in open_names} <= {str(c) for c in covered}
|
||||
|
||||
|
||||
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."""
|
||||
@@ -1486,7 +1512,7 @@ def enforce_release_gates(db: Session, wp_id: Optional[str], data: Optional[dict
|
||||
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:
|
||||
if open_names and not _urgent_constraint_override(data, open_names):
|
||||
raise HTTPException(status_code=409, detail={
|
||||
"message": "Open constraints block release", "open": open_names,
|
||||
})
|
||||
@@ -1633,13 +1659,35 @@ def upsert_wp(body: WpIn, background_tasks: BackgroundTasks, user: models.User =
|
||||
ov = gate_override(body.data)
|
||||
if ov and _released(wp.status) and not _released(old_status):
|
||||
blockers = blocking_predecessors(db, wp.id, body.data)
|
||||
_ov_constraints = [str(c) for c in ov.get("constraints") or [] if c]
|
||||
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]})
|
||||
"blocking": [b["number"] or b["id"] for b in blockers],
|
||||
"constraints": _ov_constraints})
|
||||
# CR-015: holds and releases are history, not just state. Entering or leaving
|
||||
# the hold branch gets its own audit line with the actor and the reason the
|
||||
# browser recorded in data.holds - `status_changed` alone says from/to but
|
||||
# not why, and the WHY is what a notice of delay is built from.
|
||||
if not is_new and old_status != wp.status and "Issue" in (old_status, wp.status):
|
||||
_hlds = [h for h in ((wp.data or {}).get("holds") or []) if isinstance(h, dict)]
|
||||
if wp.status == "Issue":
|
||||
_h = next((h for h in reversed(_hlds) if not h.get("released")), None)
|
||||
log_event(db, user, "hold_logged", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id),
|
||||
detail={"from": old_status,
|
||||
"constraint": str((_h or {}).get("constraint") or "")[:200],
|
||||
"reason": str((_h or {}).get("details") or "")[:300]})
|
||||
else:
|
||||
_h = next((h for h in reversed(_hlds) if h.get("released")), None)
|
||||
log_event(db, user, "hold_released", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id),
|
||||
detail={"to": wp.status,
|
||||
"constraint": str((_h or {}).get("constraint") or "")[:200],
|
||||
"reason": str((_h or {}).get("details") or "")[:300]})
|
||||
# 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):
|
||||
if not is_new and (_released(old_status) or old_status == HOLD_STATUS):
|
||||
reopened = reopened_critical(old_data, wp.data)
|
||||
if reopened:
|
||||
log_event(db, user, "constraint_reopened", "wp", wp.id, project_id=wp.project_id,
|
||||
@@ -2386,6 +2434,22 @@ def set_wp_status(wp_id: str, body: StatusIn, user: models.User = Depends(auth.g
|
||||
if old_status != body.status:
|
||||
log_event(db, user, "status_changed", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id), detail={"from": old_status, "to": body.status})
|
||||
if "Issue" in (old_status, body.status):
|
||||
_hlds = [h for h in ((wp.data or {}).get("holds") or []) if isinstance(h, dict)]
|
||||
if body.status == "Issue":
|
||||
_h = next((h for h in reversed(_hlds) if not h.get("released")), None)
|
||||
log_event(db, user, "hold_logged", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id),
|
||||
detail={"from": old_status,
|
||||
"constraint": str((_h or {}).get("constraint") or "")[:200],
|
||||
"reason": str((_h or {}).get("details") or "")[:300]})
|
||||
else:
|
||||
_h = next((h for h in reversed(_hlds) if h.get("released")), None)
|
||||
log_event(db, user, "hold_released", "wp", wp.id, project_id=wp.project_id,
|
||||
summary=(wp.number or wp.subject or wp.id),
|
||||
detail={"to": body.status,
|
||||
"constraint": str((_h or {}).get("constraint") or "")[:200],
|
||||
"reason": str((_h or {}).get("details") or "")[:300]})
|
||||
db.commit()
|
||||
db.refresh(wp)
|
||||
return wp.to_dict()
|
||||
|
||||
Reference in New Issue
Block a user