R2 hold-model correctness fixes (PR #56 review: #2,3,4,5,8,11)

[CST-100]

Fixes a cluster of interrelated correctness bugs in the quality-hold / work-order-completion model found in the PR #56 review (findings #2, #3, #4, #5, #8, #11). Branch is off the current tip of features/issues-exec-parts-r2 (contains the already-committed mechanical XSS/500/link fixes at 7b8b232); this PR targets that branch, not devel/master. One commit per finding, plus a cleanup commit.

Full suite: 788 passed, 1 skipped. ruff check src/ clean.

---

F2 — WO could close COMPLETED over an open, undispositioned NC

Wrong: check_instance_completion inferred close-out from hold_state.wo_blocked alone, and OP auto-complete consulted only blockers_for_complete. Two reachable states closed a WO over an open containment-bearing NC: (a) a "resolve by" boundary bound to an OP/terminal row (lands in start_blocked, never consulted); (b) an op/step hold set onto an already-terminal row nothing revisits.
Fix: the final close-out gate is now authoritative on the issue set — blocking_issues_for_instance (any non-advisory, undispositioned, open issue on the WO). The OP auto-complete gate folds blockers_for_start alongside blockers_for_complete. Dispositioned/advisory issues stay excluded, so legitimate completion is never deadlocked.
Tests: test_wo_stays_open_under_boundary_bound_to_terminal_step, test_wo_stays_open_under_op_hold_on_terminal_op, test_op_does_not_auto_complete_over_bound_hold_on_op_row (all verified failing on the old gate).

F3 — a hold bound to the wrong/no work order reported success but blocked nothing

Wrong: core/holds filters on Issue.procedure_instance_id, but the create/bind seams never validated it — an issue could name a different WO than its anchor step, or an MCP bind could point at a foreign execution's step.
Fix: issues.create / MCP _raise_issue derive procedure_instance_id from a step anchor when unset and reject an anchor that names a different WO (bare drafts with no anchor + no WO still allowed — the WO is linked later). MCP _bind_issue_hold adopts the target WO when the issue names none, rejects a different one.
Tests: test_create_issue_rejects_anchor_in_different_wo, test_create_issue_derives_wo_from_anchor, MCP test_bind_issue_hold_rejects_foreign_work_order, test_bind_issue_hold_adopts_work_order_when_unset.

F4 — held/sequence-gated rows rendered an active COMPLETE the server then 400'd

Wrong: _step_row.html and _dockbar.html derived "held" from step_holding_ncs (raised/containment) only, ignoring bound "resolve by" holds and sequence/redline gates.
Fix at altitude: the route computes one authoritative answer per step execution — complete_gate_by_se (core.complete_blockers: held scope + sequence) and skip_gate_by_se (core.skip_blockers: held scope + redline). A new ex.complete_control macro renders the button or the absent-control blocker line from that single field; both templates use it, and SKIP hides on skip_gate. Templates no longer re-derive gating. Removed the dead step_complete_blockers/step_start_blockers context keys.
Test: test_bound_hold_row_renders_no_active_complete (+ existing test_held_step_renders_blocker_line_not_complete_button / test_signed_disposition_restores_controls still green).

F5 — SKIP bypassed the redline-rework gate ⚑ decision flagged

Wrong: skip_step checked only containment/scope holds, so after a disposition was signed COMPLETE still refused ("Waiting on redline op") while SKIP marked the host step terminal SKIPPED — stranding authorized rework.
Fix: added core.skip_blockers = held scope + the redline-class blockers only, deliberately splitting out strict_sequence/OP-dependency ordering (skipping legitimately need not wait on predecessors). skip_step gates on it.
⚑ Decision for confirmation: I implemented the safe reading — an open redline-rework op blocks SKIP. If product intent is that SKIP should be allowed to abandon a host step and its rework, revert this gate.
Tests: test_skip_blocked_by_open_redline_after_disposition, test_skip_not_blocked_by_strict_sequence_predecessor.

F8 — containment_step_id couldn't be cleared; foreign step ids weren't validated ⚑ decision flagged

Wrong: set_containment (API + MCP) applied containment_step_id only when non-null (a mis-bound boundary was immutable) and never checked the posted step belonged to the issue's WO.
Fix: explicit-presence semantics (model_fields_set / key-in-args) — an omitted field leaves the boundary untouched, an explicit null clears it. A non-null boundary must belong to the issue's WO (seeds the WO on an unlinked issue, rejected when it names a different one).
⚑ Decision for confirmation (clear semantics): I chose a documented explicit-null clear over the disposition_type empty-string sentinel, because containment_step_id is an int field and an explicit JSON null is the natural clear signal. devel's explicit-null rejection was for non-nullable PATCH fields; this field is nullable, so a clear is appropriate.
Tests: test_set_containment_clears_boundary_with_explicit_null, test_set_containment_rejects_foreign_step (+ MCP mirror).

F11 — HOLDING link pointed at the dissolved operations tab

Wrong: holding_readout.exec_href built /executions/{id}?tab=operations&op=N; the operations tab is gone and the document route ignores tab.
Fix: emit /executions/{id}?op={order}; execdoc.js initExecDoc reads ?op= on load and calls the existing jumpToStep(order) (expands the collapsed OP card).
Test: test_holding_readout_links_to_document_op_param.

Cleanup (one fact, one home)

• execution_flow._status_value was a copy of holds._val → alias the canonical core/holds helper.
• routes.py inlined the terminal-status set twice → reuse execution_flow.TERMINAL_STEP_STATUSES.
• dropped the dead context['users'] query (execution templates read the identical active_users).
• Not consolidated: none deferred — all the listed reuse targets were addressed.

---

🤖 Generated with Claude Code