Close gate bypasses found in the pre-implementation review

- Work Order: copy the Packaging Specification from the Sales Order Item
  (walking back through the Production Plan for sub-assembly orders), and
  re-derive release blockers on submit instead of trusting a stored status.
  Tooling readiness is now derived and enforced (B6/M1/M2).
- Whitelisted operations no longer save with ignore_permissions: callers
  need the right role and write permission on the document (B6).
- Dispatch Gate matches final/outgoing inspections by exact batch and needs
  a batch on every controlled line; Reserved mode needs stock reservation (M3).
- Returns check read a field Delivery Note Item doesn't have.
- Drop the credit-note rule that blocked legitimate partial credits; ERPNext
  already caps cumulative returned qty (m5).
- Billing recompute and Supply Close query only this order's documents (m6).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

makap_backend/makap_backend/controls.py

@@ -1,5 +1,7 @@
 import frappe
 
+from makap_backend.makap_backend import work_order_readiness
+
 
 def _is_manufactured_item(item_code):
     if not item_code or not frappe.db.exists("Item", item_code):
@@ -93,24 +95,17 @@ def validate_sales_order(doc, method=None):
 
 
 def validate_work_order(doc, method=None):
+    work_order_readiness.apply_specification(doc)
+    blockers = work_order_readiness.release_blockers(doc)
+
     if doc.docstatus != 1:
         return
 
-    if doc.makap_manufacturing_readiness_status not in {"Ready", "Released"}:
+    # Re-derived here rather than trusting the stored readiness status, so a
-        frappe.throw("Submitted Work Order requires Manufacturing Readiness Status Ready or Released.")
+    # stale "Ready" set before the spec or tooling changed can't release a job.
-
+    if blockers:
-    if doc.makap_packaging_specification:
+        frappe.throw("Work Order cannot be released:\n" + "\n".join(blockers))
-        spec_status = frappe.db.get_value(
+    doc.makap_manufacturing_readiness_status = "Released"
-            "Packaging Specification", doc.makap_packaging_specification, "specification_status"
-        )
-        if spec_status != "Approved for Manufacture":
-            frappe.throw("Work Order requires a Packaging Specification approved for manufacture.")
-
-    if doc.makap_bom_compatibility_status not in {"Compatible", "Not Applicable"}:
-        frappe.throw("Work Order requires a compatible BOM decision before submission.")
-
-    if doc.makap_process_compatibility_status not in {"Compatible", "Not Applicable"}:
-        frappe.throw("Work Order requires a compatible process decision before submission.")
 
 
 def validate_quality_inspection(doc, method=None):
@@ -134,15 +129,35 @@ def _is_controlled_item(item_code):
     return control_class in {"Controlled", "High Traceability"}
 
 
-def _latest_quality_disposition(item_code, batch_no=None):
+DISPATCH_GATE_STAGES = ["Final", "Outgoing"]
-    """Best-effort read of the most recent relevant Quality Inspection for this
+
-    item/batch. Real batch-population matching (M8 section 8/9) is intentionally
+
-    left as a follow-up once Makap's actual QI-to-batch linkage in this bench is
+def _line_batches(line):
-    confirmed — this covers the common case where batch_no is captured on the QI.
+    """Batches on a stock line, whether held directly or in a Serial and Batch Bundle."""
+    if line.get("batch_no"):
+        return [line.batch_no]
+    if line.get("serial_and_batch_bundle"):
+        return frappe.get_all(
+            "Serial and Batch Entry",
+            filters={"parent": line.serial_and_batch_bundle, "batch_no": ["is", "set"]},
+            pluck="batch_no",
+            distinct=True,
+        )
+    return []
+
+
+def _latest_quality_disposition(item_code, batch_no):
+    """Most recent submitted final/outgoing Quality Inspection for this exact batch.
+
+    Matching on the batch is mandatory: an item-only match would let a pass on
+    one roll release a different roll that was never inspected.
     """
-    filters = {"item_code": item_code, "docstatus": 1}
+    filters = {
-    if batch_no:
+        "item_code": item_code,
-        filters["batch_no"] = batch_no
+        "batch_no": batch_no,
+        "docstatus": 1,
+        "makap_quality_gate_stage": ["in", DISPATCH_GATE_STAGES],
+    }
     rows = frappe.get_all(
         "Quality Inspection",
         filters=filters,
@@ -164,6 +179,8 @@ def _compute_dispatch_gate(doc):
     if not doc.customer:
         blockers.append("No Customer on Delivery Note.")
 
+    reservation_enabled = frappe.db.get_single_value("Stock Settings", "enable_stock_reservation")
+
     for line in doc.get("items") or []:
         if not _is_controlled_item(line.item_code):
             continue
@@ -173,19 +190,27 @@ def _compute_dispatch_gate(doc):
         if not line.makap_dispatch_allocation_mode:
             blockers.append(prefix + "Dispatch Allocation Mode (Reserved/Pooled) not set.")
 
-        if line.makap_dispatch_allocation_mode == "Reserved" and not line.makap_stock_reservation_entry:
+        if line.makap_dispatch_allocation_mode == "Reserved":
-            blockers.append(prefix + "Reserved mode requires a Stock Reservation Entry reference.")
+            if not reservation_enabled:
+                blockers.append(prefix + "Reserved mode needs Stock Reservation enabled in Stock Settings; use Pooled.")
+            elif not line.makap_stock_reservation_entry:
+                blockers.append(prefix + "Reserved mode requires a Stock Reservation Entry reference.")
 
         if not line.qty or line.qty <= 0:
             blockers.append(prefix + "Delivery quantity must be greater than zero.")
 
-        disposition = _latest_quality_disposition(line.item_code, getattr(line, "batch_no", None))
+        batches = _line_batches(line)
-        if not disposition or disposition.makap_quality_eligibility != "Eligible":
+        if not batches:
-            blockers.append(prefix + "No current Quality Eligibility = Eligible found for this item/batch.")
+            blockers.append(prefix + "Controlled stock must be dispatched by batch/roll; no batch selected.")
-        elif disposition.makap_quality_disposition not in {"Release", None, ""}:
+        for batch in batches:
-            blockers.append(
+            disposition = _latest_quality_disposition(line.item_code, batch)
-                prefix + f"Latest Quality Inspection disposition is '{disposition.makap_quality_disposition}', not Release."
+            if not disposition or disposition.makap_quality_eligibility != "Eligible":
-            )
+                blockers.append(prefix + f"Batch {batch} has no final Quality Inspection marked Eligible.")
+            elif disposition.makap_quality_disposition not in {"Release", None, ""}:
+                blockers.append(
+                    prefix
+                    + f"Batch {batch}: latest final inspection is '{disposition.makap_quality_disposition}', not Release."
+                )
 
     if doc.makap_dispatch_material_accountability_status != "Cleared":
         blockers.append("Dispatch Material Accountability is not Cleared.")
@@ -219,7 +244,8 @@ def validate_delivery_note(doc, method=None):
         # directly back into a dispatchable FG Released location, and require a
         # fresh Quality disposition before the stock is reusable (M8-FR-072/073/074).
         if doc.is_return:
-            if not _returns_hold_warehouse(line.warehouse) and not _returns_hold_warehouse(line.t_warehouse):
+            # On a return Delivery Note, line.warehouse is where goods come back in.
+            if not _returns_hold_warehouse(line.warehouse):
                 frappe.throw(
                     f"Return item {line.item_code} must be received into a warehouse flagged "
                     "as Returns Hold, not directly into a dispatchable location."
@@ -291,12 +317,6 @@ def _recompute_billing_and_collection(sales_order_name):
 
     delivered_unbilled = bool(so.per_delivered and so.per_delivered > 0 and (so.per_billed or 0) < so.per_delivered)
 
-    invoices = frappe.get_all(
-        "Sales Invoice",
-        filters={"docstatus": 1},
-        fields=["name", "outstanding_amount", "grand_total", "is_return"],
-        or_filters=None,
-    )
     # Sales Invoice doesn't carry sales_order directly at header level in all cases;
     # use the child table linkage to scope invoices to this Sales Order.
     linked_invoice_names = frappe.get_all(
@@ -305,7 +325,15 @@ def _recompute_billing_and_collection(sales_order_name):
         pluck="parent",
         distinct=True,
     )
-    linked_invoices = [i for i in invoices if i.name in set(linked_invoice_names)]
+    linked_invoices = (
+        frappe.get_all(
+            "Sales Invoice",
+            filters={"name": ["in", linked_invoice_names], "docstatus": 1},
+            fields=["name", "outstanding_amount", "grand_total", "is_return"],
+        )
+        if linked_invoice_names
+        else []
+    )
 
     if not linked_invoices:
         collection_status = "Not Yet Due"
@@ -335,34 +363,9 @@ def _sales_orders_on_invoice(doc):
 
 
 def validate_sales_invoice(doc, method=None):
-    if doc.is_return and doc.return_against:
+    # M8-BR-14 (no double credit for one return) is enforced by ERPNext's own
-        # M8-BR-14: one physical return event = one stock consequence. Block a second
+    # return validation, which caps cumulative credited qty at the invoiced qty.
-        # Credit Note crediting the same original invoice for a controlled item.
+    # A second, partial Credit Note for the same item is legitimate and allowed.
-        for line in doc.get("items") or []:
-            if not _is_controlled_item(line.item_code):
-                continue
-            existing = frappe.get_all(
-                "Sales Invoice",
-                filters={
-                    "is_return": 1,
-                    "return_against": doc.return_against,
-                    "docstatus": 1,
-                    "name": ["!=", doc.name],
-                },
-                pluck="name",
-            )
-            if existing:
-                credited_items = frappe.get_all(
-                    "Sales Invoice Item",
-                    filters={"parent": ["in", existing], "item_code": line.item_code},
-                    pluck="name",
-                )
-                if credited_items:
-                    frappe.throw(
-                        f"Item {line.item_code} against {doc.return_against} already has a Credit "
-                        f"Note ({existing[0]}) — this looks like a duplicate return credit."
-                    )
-
     for line in doc.get("items") or []:
         if not _is_controlled_item(line.item_code):
             continue

makap_backend/makap_backend/doctype/order_closure_and_margin_view/order_closure_and_margin_view.py

@@ -26,6 +26,14 @@ class OrderClosureAndMarginView(Document):
 
     @frappe.whitelist()
     def recompute(self):
+        self.check_permission("write")
+        self._recompute()
+        return {
+            "operating_close": self.operating_close,
+            "primary_blocker": self.primary_blocker,
+        }
+
+    def _recompute(self):
         """Best-effort recomputation from linked M4-M7 records already in the bench.
         Cost/Margin fields are intentionally left as manual entry — deriving those
         correctly needs a real pass over Stock Ledger/GL/Landed Cost Voucher data,
@@ -102,24 +110,32 @@ class OrderClosureAndMarginView(Document):
         )
         self.quality_close = "Open" if open_quality else "Closed"
 
-        # Supply Close — first-pass only. Real M7 lineage (Material Request ->
+        # Supply Close — purchases raised for this order through its Production
-        # Production Plan -> Sales Order) isn't traversed here yet; this checks
+        # Plan(s): Sales Order -> Production Plan -> Material Request -> PO.
-        # Purchase Order Items sharing this order's items as a rough proxy and
+        plans = frappe.get_all(
-        # should be replaced once the actual M7 linkage is confirmed against data.
+            "Production Plan Sales Order", filters={"sales_order": so_name}, pluck="parent", distinct=True
-        item_codes = frappe.get_all(
+        )
-            "Sales Order Item", filters={"parent": so_name}, pluck="item_code", distinct=True
+        requests = (
+            frappe.get_all(
+                "Material Request Item",
+                filters={"production_plan": ["in", plans], "docstatus": 1},
+                pluck="parent",
+                distinct=True,
+            )
+            if plans
+            else []
         )
         at_risk_po = (
             frappe.get_all(
                 "Purchase Order Item",
                 filters={
-                    "item_code": ["in", item_codes],
+                    "material_request": ["in", requests],
                     "makap_supplier_confirmation_status": ["in", ["At Risk", "Declined"]],
                     "docstatus": 1,
                 },
                 limit=1,
             )
-            if item_codes
+            if requests
             else []
         )
         self.supply_close = "Open" if at_risk_po else "Closed"
@@ -127,11 +143,7 @@ class OrderClosureAndMarginView(Document):
         self._derive_operating_close()
         self.last_recomputed_by = frappe.session.user
         self.last_recomputed_on = frappe.utils.now_datetime()
-        self.save(ignore_permissions=True)
+        self.save()
-        return {
-            "operating_close": self.operating_close,
-            "primary_blocker": self.primary_blocker,
-        }
 
 
 @frappe.whitelist()
@@ -139,7 +151,7 @@ def get_or_create(sales_order):
     if not frappe.db.exists("Order Closure And Margin View", sales_order):
         doc = frappe.get_doc(
             {"doctype": "Order Closure And Margin View", "sales_order": sales_order}
-        ).insert(ignore_permissions=True)
+        ).insert()
     else:
         doc = frappe.get_doc("Order Closure And Margin View", sales_order)
     doc.recompute()

makap_backend/makap_backend/operations.py

@@ -1,4 +1,12 @@
 import frappe
+from frappe.utils import cint
+
+from makap_backend.makap_backend import work_order_readiness
+
+READINESS_ROLES = ("Production Authority", "Manufacturing Manager")
+MATERIAL_CHECK_ROLES = ("Production Authority", "Manufacturing Manager", "Materials Controller", "Stock Manager")
+DISPOSITION_ROLES = ("Quality Manager",)
+CONCESSION_ROLES = ("Concession Authority",)
 
 
 def _require_doc(doctype, name):
@@ -7,53 +15,52 @@ def _require_doc(doctype, name):
     return frappe.get_doc(doctype, name)
 
 
+def _record(doc, values):
+    """Store derived evidence under the caller's own permissions.
+
+    Submitted documents can't be re-saved, so evidence on them is written
+    directly; the caller has already passed the role and write checks.
+    """
+    doc.check_permission("write")
+    if doc.docstatus == 1:
+        doc.db_set(values)
+    else:
+        doc.update(values)
+        doc.save()
+
+
 @frappe.whitelist()
 def evaluate_work_order_readiness(work_order_name, save=True):
-    """Derive bounded readiness fields from standard Work Order/BOM/specification data."""
+    """Derive readiness from the same rules the Work Order submit gate enforces."""
+    frappe.only_for(READINESS_ROLES)
     work_order = _require_doc("Work Order", work_order_name)
-    blockers = []
+    blockers = work_order_readiness.release_blockers(work_order)
+    status = "Blocked" if blockers else "Ready"
 
-    spec_status = None
+    if cint(save):
-    if work_order.makap_packaging_specification:
+        _record(
-        spec_status = frappe.db.get_value(
+            work_order,
-            "Packaging Specification", work_order.makap_packaging_specification, "specification_status"
+            {
+                "makap_manufacturing_readiness_status": status,
+                "makap_manufacturing_readiness_blockers": "\n".join(blockers),
+                "makap_tooling_readiness_status": work_order.makap_tooling_readiness_status,
+                "makap_readiness_checked_by": frappe.session.user,
+                "makap_readiness_checked_on": frappe.utils.now_datetime(),
+            },
         )
-        if spec_status != "Approved for Manufacture":
-            blockers.append("Packaging Specification is not Approved for Manufacture")
-    elif work_order.production_item:
-        requires_spec = frappe.db.get_value(
-            "Item", work_order.production_item, "makap_requires_packaging_specification"
-        )
-        if requires_spec:
-            blockers.append("Production Item requires a Packaging Specification")
-
-    if not work_order.bom_no:
-        blockers.append("No BOM is selected")
-    elif work_order.makap_bom_compatibility_status not in {"Compatible", "Not Applicable"}:
-        blockers.append("BOM compatibility is not confirmed")
-
-    if work_order.makap_process_compatibility_status not in {"Compatible", "Not Applicable"}:
-        blockers.append("Process compatibility is not confirmed")
-
-    work_order.makap_manufacturing_readiness_status = "Blocked" if blockers else "Ready"
-    work_order.makap_manufacturing_readiness_blockers = "\n".join(blockers)
-    work_order.makap_readiness_checked_by = frappe.session.user
-    work_order.makap_readiness_checked_on = frappe.utils.now_datetime()
-    if save:
-        work_order.save(ignore_permissions=True)
-        frappe.db.commit()
 
     return {
         "work_order": work_order.name,
-        "status": work_order.makap_manufacturing_readiness_status,
+        "status": status,
         "blockers": blockers,
-        "specification_status": spec_status,
+        "tooling_status": work_order.makap_tooling_readiness_status,
     }
 
 
 @frappe.whitelist()
 def evaluate_material_availability(work_order_name, save=True):
     """Compare standard Work Order required quantities with standard Bin actual quantity."""
+    frappe.only_for(MATERIAL_CHECK_ROLES)
     work_order = _require_doc("Work Order", work_order_name)
     shortages = []
     partially_available = False
@@ -82,13 +89,16 @@ def evaluate_material_availability(work_order_name, save=True):
     else:
         status = "Available"
 
-    work_order.makap_material_availability_status = status
+    if cint(save):
-    work_order.makap_material_availability_checked_by = frappe.session.user
+        _record(
-    work_order.makap_material_availability_checked_on = frappe.utils.now_datetime()
+            work_order,
-    work_order.makap_material_availability_blocker_summary = "\n".join(shortages)
+            {
-    if save:
+                "makap_material_availability_status": status,
-        work_order.save(ignore_permissions=True)
+                "makap_material_availability_checked_by": frappe.session.user,
-        frappe.db.commit()
+                "makap_material_availability_checked_on": frappe.utils.now_datetime(),
+                "makap_material_availability_blocker_summary": "\n".join(shortages),
+            },
+        )
 
     return {"work_order": work_order.name, "status": status, "shortages": shortages}
 
@@ -96,7 +106,10 @@ def evaluate_material_availability(work_order_name, save=True):
 @frappe.whitelist()
 def apply_quality_disposition(quality_inspection_name, disposition, quantity=None, reason=None):
     """Record bounded quality disposition evidence on a standard Quality Inspection."""
+    frappe.only_for(CONCESSION_ROLES + DISPOSITION_ROLES if disposition == "Concession" else DISPOSITION_ROLES)
     inspection = _require_doc("Quality Inspection", quality_inspection_name)
+    if inspection.docstatus != 0:
+        frappe.throw("Disposition can only be recorded on a draft Quality Inspection; amend it to change a submitted one.")
     allowed = {"Pending", "Release", "Hold", "Reject", "Rework", "Scrap", "Concession"}
     if disposition not in allowed:
         frappe.throw("Invalid quality disposition.")
@@ -114,14 +127,17 @@ def apply_quality_disposition(quality_inspection_name, disposition, quantity=Non
         inspection.makap_quality_eligibility = "Eligible"
     elif disposition in {"Hold", "Reject", "Rework", "Scrap"}:
         inspection.makap_quality_eligibility = "Hold" if disposition == "Hold" else "Ineligible"
-    inspection.save(ignore_permissions=True)
+    inspection.check_permission("write")
-    frappe.db.commit()
+    inspection.save()
     return {"quality_inspection": inspection.name, "disposition": disposition}
 
 
 @frappe.whitelist()
 def get_supply_assurance_snapshot(item_code, warehouse=None):
     """Return a derived supply view from standard stock, material requests, and purchase orders."""
+    for doctype in ("Bin", "Material Request", "Purchase Order"):
+        if not frappe.has_permission(doctype, "read"):
+            frappe.throw(f"You do not have permission to read {doctype}.", frappe.PermissionError)
     bins = frappe.get_all(
         "Bin",
         filters={"item_code": item_code, **({"warehouse": warehouse} if warehouse else {})},

makap_backend/makap_backend/work_order_readiness.py

@@ -0,0 +1,107 @@
+"""Work Order release rules, shared by the readiness check and the submit gate.
+
+Both callers run the same rules so a readiness result can never disagree with
+what submission actually enforces.
+"""
+
+import frappe
+
+RELEASABLE_SPEC_STATUS = "Approved for Manufacture"
+READY_CYLINDER_STATUS = "Ready"
+COMPATIBLE = {"Compatible", "Not Applicable"}
+
+
+def _sales_order_item_for(work_order):
+    """Find the Sales Order Item a Work Order serves.
+
+    Sub-assembly Work Orders (printed roll, laminated roll) carry no
+    sales_order_item of their own, so walk back through the Production Plan
+    to the finished-goods row that does.
+    """
+    if work_order.get("sales_order_item"):
+        return work_order.sales_order_item
+
+    plan_item = work_order.get("production_plan_item")
+    if not plan_item and work_order.get("production_plan_sub_assembly_item"):
+        plan_item = frappe.db.get_value(
+            "Production Plan Sub Assembly Item",
+            work_order.production_plan_sub_assembly_item,
+            "production_plan_item",
+        )
+    if plan_item:
+        so_item = frappe.db.get_value("Production Plan Item", plan_item, "sales_order_item")
+        if so_item:
+            return so_item
+
+    if work_order.get("sales_order") and work_order.get("production_item"):
+        matches = frappe.get_all(
+            "Sales Order Item",
+            filters={"parent": work_order.sales_order, "item_code": work_order.production_item},
+            pluck="name",
+        )
+        if len(matches) == 1:
+            return matches[0]
+    return None
+
+
+def apply_specification(work_order):
+    """Copy the Packaging Specification from the Sales Order Item onto the Work Order."""
+    if work_order.get("makap_packaging_specification"):
+        return
+    so_item = _sales_order_item_for(work_order)
+    if not so_item:
+        return
+    spec = frappe.db.get_value("Sales Order Item", so_item, "makap_packaging_specification")
+    if not spec:
+        return
+    work_order.makap_packaging_specification = spec
+    work_order.makap_specification_revision = frappe.db.get_value("Packaging Specification", spec, "revision")
+    work_order.makap_sales_order_item_reference = so_item
+    work_order.makap_customer = frappe.db.get_value("Packaging Specification", spec, "customer")
+
+
+def tooling_status(spec_name):
+    """Return (status, blockers) for the print cylinders/plates a specification needs."""
+    if not spec_name:
+        return "Not Required", []
+    if not frappe.db.get_value("Packaging Specification", spec_name, "print_tooling_required"):
+        return "Not Required", []
+
+    cylinders = frappe.get_all(
+        "Print Cylinder",
+        filters={"packaging_specification": spec_name, "status": ["!=", "Retired"]},
+        fields=["name", "status"],
+    )
+    if not cylinders:
+        return "Blocked", ["Print tooling is required but no cylinders or plates are registered"]
+    not_ready = [c.name for c in cylinders if c.status != READY_CYLINDER_STATUS]
+    if not_ready:
+        return "Blocked", [f"Tooling not Ready: {', '.join(not_ready)}"]
+    return "Ready", []
+
+
+def release_blockers(work_order):
+    """Everything that stops this Work Order from being released to the floor."""
+    blockers = []
+    spec = work_order.get("makap_packaging_specification")
+
+    if spec:
+        if frappe.db.get_value("Packaging Specification", spec, "specification_status") != RELEASABLE_SPEC_STATUS:
+            blockers.append("Packaging Specification is not Approved for Manufacture")
+    elif work_order.get("production_item") and frappe.db.get_value(
+        "Item", work_order.production_item, "makap_requires_packaging_specification"
+    ):
+        blockers.append("Production Item requires a Packaging Specification")
+
+    if not work_order.get("bom_no"):
+        blockers.append("No BOM is selected")
+    elif work_order.get("makap_bom_compatibility_status") not in COMPATIBLE:
+        blockers.append("BOM compatibility is not confirmed")
+
+    if work_order.get("makap_process_compatibility_status") not in COMPATIBLE:
+        blockers.append("Process compatibility is not confirmed")
+
+    tooling, tooling_blockers = tooling_status(spec)
+    work_order.makap_tooling_readiness_status = tooling
+    blockers.extend(tooling_blockers)
+    return blockers