diff --git a/plugins/onex-dev-marketplace/.claude-plugin/marketplace.json b/plugins/onex-dev-marketplace/.claude-plugin/marketplace.json index ec229b4ed1..fc3a960d81 100644 --- a/plugins/onex-dev-marketplace/.claude-plugin/marketplace.json +++ b/plugins/onex-dev-marketplace/.claude-plugin/marketplace.json @@ -11,7 +11,7 @@ { "name": "onex", "description": "Full ONEX architecture plugin — intelligent agent routing, manifest injection, quality enforcement, polymorphic YAML agents, reusable skills, and custom slash commands. Internal dev tooling only; distinct MARKETPLACE (omninode-tools-dev) so `onex@omninode-tools-dev` never collides with the consumer-facing `onex@omninode-tools` install — the plugin name matches plugins/onex/.claude-plugin/plugin.json's own `name` field, uniqueness comes from marketplace scoping.", - "version": "2.4.17", + "version": "2.4.18", "author": { "name": "OmniNode", "email": "dev@omninode.ai" diff --git a/plugins/onex/.claude-plugin/plugin.json b/plugins/onex/.claude-plugin/plugin.json index 72bdda2606..2e153875b6 100644 --- a/plugins/onex/.claude-plugin/plugin.json +++ b/plugins/onex/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "onex", "description": "Unified ONEX architecture plugin with intelligent agent routing, manifest injection, quality enforcement, 53 polymorphic YAML agents, and 100+ reusable skills (invoked as /onex:) for comprehensive workflow automation and observability", - "version": "2.4.17", + "version": "2.4.18", "author": { "name": "OmniNode", "email": "dev@omninode.ai" diff --git a/plugins/onex/hooks/contracts/hook_inventory.yaml b/plugins/onex/hooks/contracts/hook_inventory.yaml index 5fcee8ce4b..4b6d62a52b 100644 --- a/plugins/onex/hooks/contracts/hook_inventory.yaml +++ b/plugins/onex/hooks/contracts/hook_inventory.yaml @@ -134,16 +134,22 @@ expected_hooks: event: "PreToolUse" matcher: "^mcp__linear-server__(save_issue|update_issue)$" order: 0 - ticket: "OMN-13856" + ticket: "OMN-20368" owner: "omniclaude hooks lane" purpose: > - Refuses a Linear flip to Done when the ticket carries no merged-PR - citation and no PASS node_dod_verify receipt on OCC main. Fail-closed: - no fake Done. + Refuses a Linear flip to Done, and an edit that ticks an acceptance + box, unless the ticket's OCC contract on origin/dev binds every labelled + acceptance criterion to a PASS receipt taken against the current + contract entry (OMN-20368; the guard itself is OMN-13856). A merged PR, + ticked boxes, a deploy-readback marker or an exemption label are further + conditions, never substitutes. Fail-closed: it runs in every cwd and in + lite mode, has no mask bit, and refuses when it cannot decide. enforcement: true - lite_mode_exit: true + lite_mode_exit: false mask: - gate_call: "DONE_FLIP_GUARD" + # No gate (OMN-20368): a Done is global state, so no per-machine switch + # may turn its check off. + gate_call: null bit_defined: false canary: kind: "block" @@ -158,7 +164,7 @@ expected_hooks: exit_code: 2 stdout_contains: - '"decision": "block"' - - "no_durable_evidence" + - "no_bound_dod_receipt" stdout_absent: [] - script: "pre_tool_use_ticket_creation_gate.sh" diff --git a/plugins/onex/hooks/lib/done_flip_guard.py b/plugins/onex/hooks/lib/done_flip_guard.py index 2c93a175ce..75bd8f823b 100644 --- a/plugins/onex/hooks/lib/done_flip_guard.py +++ b/plugins/onex/hooks/lib/done_flip_guard.py @@ -25,60 +25,38 @@ OR a receipt-PASS"), and a hook can only block or pass through — two independent hooks each pass on their own criterion and the fabricated Done slips through. -Decision (fail-closed — the default outcome for a real Done-flip is BLOCK): - -1. Not a ``save_issue``/``update_issue`` call, or not a Done-class target state - → ALLOW (nothing to verify). -2. cancel-class target state (``canceled/duplicate/won't do``) → ALLOW. -2b. Unchecked acceptance-criteria checkbox gate (OMN-15030): if the ticket's - current description contains any unchecked GFM task-list box (``- [ ]``) - → BLOCK, unconditionally — no later evidence path (merged PR, OCC receipt, - exempt label) waives this. Every later path proves *some* evidence exists; - none of them prove the ticket's own stated acceptance criteria were met. - OMN-13991 is the concrete incident this closes: a genuinely merged, cited, - implementing PR still under-delivered against the ticket's own DoD text, - and passed every existing presence-check because "merged" was treated as - sufficient. See :func:`find_unchecked_acceptance_boxes`. -2a. Durable evidence path C — deploy-readback marker (OMN-14792): a - runtime-deploy ticket whose DoD is a live readback, not a merged product PR - (``node_dod_verify`` structurally skips such tickets — - ``reference_dod_verify_cannot_close_deploy_tickets`` — and they close via an - operator deliberate-Done). When a ``deploy-readback-proven: `` marker with real evidence is present, the merge check is SCOPED to - DoD-*implementing* PRs (``verify_implementing``): full-URL / resolvable bare - ``#N`` citations, EXCLUDING scratch/throwaway/live-mint-annotated PRs and - unresolvable merge-chain-narrative ``#N`` refs. No unmerged implementing PR - → ALLOW; an unmerged implementing PR still BLOCKS (the marker is not a - blanket bypass — the OMN-14641 lesson). This closes the OMN-14437 false block - where the guard treated a closed scratch live-mint PR and bare narrative - numbers as blocking DoD evidence. -3. Durable evidence path A — merged PR: if the ticket cites (or links via a - Linear attachment) any *product* PR and every one is ``MERGED`` → ALLOW. If - any is open / unmerged → BLOCK. (A "superseded-by-merged-sibling" close is a - merged-PR citation and is accepted here.) ``onex_change_control`` evidence / - OCC-receipt PRs are WEAK signals and are filtered out — they never gate a - product Done in either direction (OMN-14641, deliverable 3). -3a. Exemption carve-out — an explicit ``close-if-done`` label / frontmatter - (covers decision-only tickets and epic ALL_CHILDREN_DONE roll-ups) → ALLOW, - but ONLY when no product PR is cited/linked. OMN-14641: the label was - previously a blanket merge-check bypass, so a ticket carrying it flipped Done - with its linked product PR still OPEN (the OMN-14582 false-Done). The label - can no longer waive an open cited/linked PR — that path BLOCKS at step 3. -4. Durable evidence path B — the no-PR bar (OMN-13856), reached only when the - ticket cites no PR at all. Read off ``origin/dev`` of the local - onex_change_control clone: the ticket's contract ``contracts/.yaml`` - binds EVERY labelled acceptance criterion of the ticket through ``binds_ac``, - and for each criterion a binding item has a PASS receipt under - ``drift/dod_receipts//`` that names the subject (ticket, item, a - declared check type), the environment (``target_identity`` or - ``working_dir``) and a read time (``run_timestamp``) inside the freshness - window, attested by a verifier other than its runner, with a non-empty - ``probe_stdout`` → ALLOW. This is the evidence closer's per-criterion bar; - see ``no_pr_bound_evidence``. Until OMN-13856 ANY PASS receipt for the - ticket passed here, whatever it proved and however old it was. -5. Otherwise → BLOCK. A Done-flip with no merged-PR citation and no bound - evidence is refused; if the evidence cannot be resolved at all, the guard - STILL BLOCKS (never fail-open on a fake-Done — design requirement 4). +Decision (OMN-20368 — fail-closed; the ONE sufficient condition for Done is +the bound-receipt bar, and every other path is a further condition): + +1. Not a ``save_issue``/``update_issue`` call → ALLOW. A state passed by id + (not by name) is REFUSED: it cannot be classified without a network read. +2. Cancel-class target state (``canceled/duplicate/won't do``) → ALLOW. +3. Acceptance-box tick gate (OMN-20368), for every non-Done edit that writes a + description (``description`` or ``patch``): a box checked in the result that + was not checked before is a tick, and a tick needs the bound-receipt bar + (step 7). A create with a checked box is refused. When the current + description cannot be read, every checked box counts as a tick. +4. Done: an unchecked GFM box in today's description or the written one → + BLOCK (OMN-15030). Ticking it is not the remedy; the receipt is. +5. Done: every cited / linked product PR must be merged (OMN-8375/OMN-14641), + scoped to implementing PRs under a deploy-readback marker (OMN-14792), with + OMN-15712's closed-and-uncited/superseded attachments excused. A merged PR + used to ALLOW here; since OMN-20368 it is necessary and never sufficient. +6. The close-if-done exemption label and the deploy-readback marker no longer + ALLOW anything on their own (OMN-20368). +7. The bound-receipt bar (``no_pr_bound_evidence``), on ``origin/dev`` of the + local onex_change_control clone: ``contracts/.yaml`` binds EVERY + labelled acceptance criterion — of today's description and of the one the + call writes — through ``binds_ac``, and each criterion has a binding item + with a PASS receipt that names the subject, the environment and a read + time, is attested by a verifier other than its runner, and was taken + against the current contract entry (``contract_entry_sha256``, or the + legacy whole-file ``contract_sha256``). A ticket with no merged PR also + holds its receipts to the freshness window. Anything short → BLOCK, naming + what is missing. dod_verify's NO_ACCEPTANCE_CHECKS is the shape "no item + binds the criterion", and it is refused. +8. A guard that cannot decide refuses (``main``), and the shell wrapper runs in + every cwd, in lite mode and under any hooks mask. Why ``origin/dev`` git-backed (freshness + determinism) — OMN-13857 findings: Two paths that LOOK authoritative are broken for a Done-flip gate: @@ -105,6 +83,7 @@ from __future__ import annotations +import inspect import json import os import re @@ -125,13 +104,15 @@ fetch_pr_status, is_cancel_state, is_done_state, - is_exempt, parse_deploy_readback_marker, verify, verify_implementing, ) from no_pr_bound_evidence import ( + MERGED_PR_ENVIRONMENT_FIELDS, + NO_PR_RECEIPT_MAX_AGE, BoundEvidenceVerdict, + bounded_fetch, evaluate_bound_evidence, load_ticket_occ_evidence, ) @@ -342,11 +323,7 @@ def list_ticket_receipt_fields( return [] if fetch: - _run_git( - ["fetch", "--quiet", "origin", "dev"], - cwd=occ_repo, - timeout=_GIT_FETCH_TIMEOUT_SECONDS, - ) + bounded_fetch(occ_repo) receipt_dir = f"{_RECEIPT_DIR_PREFIX}/{ticket_id}" listing = _run_git( @@ -449,11 +426,147 @@ def _default_linear_fetcher(ticket_id: str) -> dict[str, Any] | None: # Core decision # --------------------------------------------------------------------------- +# OMN-20368: a CHECKED task-list box. Every box counts, as for the unchecked +# gate above: a lane that ticks a box is asserting a criterion was met. +_CHECKED_BOX_RE = re.compile( + r"^[ \t]*(?:[-*+]|\d+[.)])[ \t]+\[[xX]\][ \t]+(\S.*)$", re.MULTILINE +) +_UUID_RE = re.compile( + r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.IGNORECASE +) + + +def _normalise_box_text(text: str) -> str: + """Box text compared across edits: Linear's backslash escapes and spacing removed.""" + return " ".join(text.replace("\\", "").split()).casefold() + + +def checked_acceptance_boxes(description: str) -> set[str]: + """The normalised text of every checked (``[x]``) task-list item. Pure.""" + return {_normalise_box_text(m) for m in _CHECKED_BOX_RE.findall(description or "")} + + +def newly_ticked_boxes(before: str | None, after: str) -> list[str]: + """Checked boxes in ``after`` that were not checked in ``before``. + + ``before`` is None when the current description could not be read; every + checked box in ``after`` then counts as newly ticked (fail closed: an edit + whose effect cannot be compared is treated as the edit that ticks). + """ + previously = checked_acceptance_boxes(before) if before is not None else set() + return sorted(checked_acceptance_boxes(after) - previously) + + +def _patch_texts(ops: list[Any]) -> str: + parts: list[str] = [] + for op in ops: + if isinstance(op, dict): + for key in ("new_string", "text"): + value = op.get(key) + if isinstance(value, str): + parts.append(value) + return "\n".join(parts) + + +def apply_description_patch(current: str, ops: list[Any]) -> str | None: + """Apply the Linear MCP ``patch`` operations to ``current``; None if any fails. + + Mirrors the MCP's documented semantics (every anchor must match exactly + once, unless ``replace_all``). A None result is not an error to the caller: + it means the guard cannot predict the edit, and it fails closed on it. + """ + text = current + for op in ops: + if not isinstance(op, dict): + return None + kind = op.get("op") + if kind == "replace": + old, new = op.get("old_string"), op.get("new_string") + if not isinstance(old, str) or not isinstance(new, str) or not old: + return None + if op.get("replace_all"): + if old not in text: + return None + text = text.replace(old, new) + else: + if text.count(old) != 1: + return None + text = text.replace(old, new, 1) + elif kind in ("insert_before", "insert_after"): + anchor, add = op.get("anchor"), op.get("text") + if not isinstance(anchor, str) or not isinstance(add, str): + return None + if text.count(anchor) != 1: + return None + joined = add + anchor if kind == "insert_before" else anchor + add + text = text.replace(anchor, joined, 1) + elif kind == "prepend": + add = op.get("text") + if not isinstance(add, str): + return None + text = add + text + elif kind == "append": + add = op.get("text") + if not isinstance(add, str): + return None + text = text + add + elif kind == "replace_range": + start, end, new = op.get("from"), op.get("to"), op.get("new_string") + if not all(isinstance(x, str) for x in (start, end, new)): + return None + assert isinstance(start, str) and isinstance(end, str) + assert isinstance(new, str) + if text.count(start) != 1: + return None + i = text.index(start) + j = text.find(end, i + len(start)) + if j < 0 or text.count(end, i + len(start)) != 1: + return None + text = text[:i] + new + text[j:] + else: + return None + return text + + +def _occ_lister(repo: Path) -> Callable[[str], list[dict[str, str]]]: + def lister(ticket_id: str) -> list[dict[str, str]]: + return list_ticket_receipt_fields(repo, ticket_id) + + return lister + + +def _accepts_merged_pr(probe: Callable[..., Any]) -> bool: + """Whether ``probe`` takes the ``merged_pr`` keyword (an injected test probe may not).""" + try: + params = inspect.signature(probe).parameters + except (TypeError, ValueError): + return False + return "merged_pr" in params or any( + p.kind is inspect.Parameter.VAR_KEYWORD for p in params.values() + ) + + +def _refuse(reason: str) -> Decision: + return Decision(False, reason) + + +def _bound_receipt_refusal(ticket_id: str, detail: str, *, why: str) -> Decision: + return _refuse( + f"no_bound_dod_receipt for {ticket_id}: {why}. The bar (OMN-20368) is " + f"contracts/{ticket_id}.yaml on {_OCC_REF} of onex_change_control binding " + "EVERY labelled acceptance criterion through `binds_ac`, each discharged " + f"by a PASS receipt under {_RECEIPT_DIR_PREFIX}/{ticket_id}/ taken " + "against the current contract entry. What is missing: " + f"{detail}. Ticked boxes, a merged PR, an evidence comment or an " + "exemption label do not substitute for it: run dod_verify, land the " + "binding and the receipts through an evidence PR, then flip." + ) + def decide( call: dict[str, Any], *, - occ_probe: Callable[[str, str], BoundEvidenceVerdict] | None = None, + occ_probe: Callable[..., BoundEvidenceVerdict] | None = None, pr_fetcher: Callable[[Any], Any] = fetch_pr_status, linear_fetcher: Callable[[str], dict[str, Any] | None] = _default_linear_fetcher, receipt_lister: Callable[[str], list[dict[str, str]]] | None = None, @@ -461,21 +574,25 @@ def decide( ) -> Decision: """Return the guard decision for a PreToolUse tool call. + OMN-20368: a Done transition is allowed ONLY when the ticket's OCC contract + binds every acceptance criterion to a PASS receipt taken against the + current contract entry (``occ_probe``). Every other evidence path that used + to ALLOW on its own (all cited PRs merged, a deploy-readback marker, a + close-if-done label, OMN-15712's superseded attachments) is now a further + condition, never a substitute. The same bar is required for an edit that + ticks an acceptance box. + All I/O boundaries are injectable so unit tests stay hermetic: - * ``occ_probe(ticket_id, description) -> BoundEvidenceVerdict`` — does - ``origin/dev`` of the OCC clone bind every acceptance criterion to a - fresh, attested receipt? Defaults to - :func:`no_pr_bound_evidence.evaluate_bound_evidence` over - :func:`no_pr_bound_evidence.load_ticket_occ_evidence` for the resolved - OCC clone. - * ``pr_fetcher(PRRef) -> PRStatus`` — GitHub PR state (default: ``gh``). - * ``linear_fetcher(ticket_id) -> issue|{}|None`` — live Linear read. - * ``receipt_lister(ticket_id) -> list[fields]`` — every OCC receipt's - parsed fields for the ticket (OMN-15712 supersession/citation check); - defaults to :func:`list_ticket_receipt_fields` bound to the resolved - OCC clone. - * ``now`` — the clock a no-PR receipt's read time is judged against - (OMN-13856); defaults to the current UTC time. + * ``occ_probe(ticket_id, description, merged_pr=bool)`` -> verdict on the + bound-receipt bar, read off ``origin/dev`` of the OCC clone. Defaults + to :func:`no_pr_bound_evidence.evaluate_bound_evidence` over one load of + :func:`no_pr_bound_evidence.load_ticket_occ_evidence`. A test probe may + take only ``(ticket_id, description)``. + * ``pr_fetcher(PRRef) -> PRStatus`` -- GitHub PR state (default: ``gh``). + * ``linear_fetcher(ticket_id) -> issue|{}|None`` -- live Linear read. + * ``receipt_lister(ticket_id) -> list[fields]`` -- OCC receipt fields for + the OMN-15712 supersession check. + * ``now`` -- the clock a no-PR receipt's read time is judged against. """ tool_name = call.get("tool_name", "") if tool_name not in _LINEAR_TOOLS: @@ -485,222 +602,246 @@ def decide( if not isinstance(params, dict): return Decision(True, "no_tool_input") - state_value = str(params.get("state") or params.get("status") or "") + ticket_id = str(params.get("id") or params.get("issueId") or "") + state_value = str(params.get("state") or params.get("status") or "").strip() + + # A state passed by id (the MCP accepts "type, name, or ID") cannot be + # classified without a network read, and an unclassified Done would slip + # past every check below. Refused: pass the state by name. + if state_value and _UUID_RE.match(state_value): + return _refuse( + f"state_by_id: state {state_value} was passed by id, so the guard " + "cannot tell whether this is a Done transition. Pass the state by " + "name (Done, In Progress, ...)." + ) - # (2) cancel-class carve-out: closing without shipping — no PR/receipt owed. if is_cancel_state(state_value): return Decision(True, "carve_out:cancel_state") + is_done = is_done_state(state_value) + + # ---- the live description, read once --------------------------------- + issue: dict[str, Any] | None = None + live_read = False + + def _live() -> dict[str, Any] | None: + nonlocal issue, live_read + if not live_read: + live_read = True + fetched = linear_fetcher(ticket_id) if ticket_id else None + issue = fetched if isinstance(fetched, dict) and fetched else None + return issue + + # ---- the bound-receipt bar, loaded once ------------------------------- + clock = now or datetime.now(UTC) + probe = occ_probe + probe_error = "" + if probe is None and ticket_id: + workspace_root = resolve_omni_home() + if workspace_root is None: + probe_error = ( + "OMNI_HOME is unset/invalid, so the local onex_change_control " + "clone cannot be resolved" + ) + else: + evidence = load_ticket_occ_evidence( + occ_repo_path(workspace_root), ticket_id + ) - # (1) not a Done-class transition — nothing to verify. - if not is_done_state(state_value): + def probe(tid: str, desc: str, merged_pr: bool = False) -> Any: + return evaluate_bound_evidence( + tid, + desc, + evidence, + clock, + max_age=None if merged_pr else NO_PR_RECEIPT_MAX_AGE, + environment_fields=( + MERGED_PR_ENVIRONMENT_FIELDS + if merged_pr + else ("target_identity", "working_dir") + ), + ) + + def _bar(descriptions: list[str], *, merged_pr: bool, why: str) -> Decision | None: + """None when every description's criteria are bound; else the refusal.""" + if not ticket_id: + return _refuse( + "no_ticket_id: a Done transition or an acceptance-box tick needs " + "the issue id so its bound receipts can be read. Pass 'id'." + ) + if probe is None: + return _bound_receipt_refusal(ticket_id, probe_error, why=why) + seen: set[str] = set() + for desc in descriptions: + if desc in seen: + continue + seen.add(desc) + if _accepts_merged_pr(probe): + verdict = probe(ticket_id, desc, merged_pr=merged_pr) + else: + verdict = probe(ticket_id, desc) + if not verdict.passed: + return _bound_receipt_refusal(ticket_id, verdict.detail, why=why) + return None + + # ---- the description this call would leave behind ---------------------- + has_description = isinstance(params.get("description"), str) + patch_ops = params.get("patch") + new_description: str | None = None + current_description: str | None = None + if has_description or isinstance(patch_ops, list): + live = _live() if ticket_id else None + current_description = ( + str(live.get("description") or "") if live is not None else None + ) + if not ticket_id: + current_description = "" # a create: nothing was checked before it + if has_description: + new_description = str(params["description"]) + elif isinstance(patch_ops, list): + applied = ( + apply_description_patch(current_description, patch_ops) + if current_description is not None + else None + ) + # An edit the guard cannot predict is judged by what it inserts. + new_description = ( + applied if applied is not None else _patch_texts(patch_ops) + ) + if applied is None: + current_description = None + + # ---- (T) acceptance-box tick gate (OMN-20368) ------------------------- + # A Done call is held to the full bar below on the description it writes, + # which is strictly stronger, so the tick gate serves every other edit. + if new_description is not None and not is_done: + ticked = newly_ticked_boxes(current_description, new_description) + if ticked: + if not ticket_id: + return _refuse( + "ac_tick_without_receipt: a new ticket cannot be created with " + f"checked acceptance boxes ({'; '.join(ticked[:3])}). Create " + "it with every box unchecked; boxes are ticked only once a " + "bound PASS dod_verify receipt exists (OMN-20368)." + ) + preview = "; ".join(t[:120] for t in ticked[:3]) + refused = _bar( + [new_description], + merged_pr=True, + why=( + f"this edit ticks {len(ticked)} acceptance box(es) ({preview}) " + "and no bound PASS receipt covers the criteria" + ), + ) + if refused is not None: + return Decision( + False, + refused.reason.replace( + "no_bound_dod_receipt", "ac_tick_without_receipt", 1 + ), + ) + + if not is_done: return Decision(True, "not_done_state") - ticket_id = str(params.get("id") or params.get("issueId") or "") - description = str(params.get("description") or "") + # ---- Done transition ------------------------------------------------- labels: list[str] = [str(x) for x in (params.get("labels") or [])] - - # Status-only updates commonly omit the description; read it live so PR - # citations and exemption labels can be evaluated. None => Linear - # unreachable: do NOT allow on that basis — fall through to the OCC receipt - # path, which reads git-backed governance, not Linear. - if not description and ticket_id: - issue = linear_fetcher(ticket_id) - if isinstance(issue, dict) and issue: - description = str(issue.get("description") or "") - if not labels: - labels = [str(x) for x in (issue.get("labels") or [])] - # Fold in the linked-PR attachment URLs (Linear GitHub integration - # links the PR as an attachment, not a `#N` body mention) so the - # merge check sees the *linked* PR even when it is uncited. This is - # the OMN-14582 false-Done shape — a label-driven close while the - # linked product PR was still OPEN (OMN-14641). - description = augment_description_with_attachments( - description, [str(x) for x in (issue.get("attachment_urls") or [])] + live = _live() if ticket_id else None + live_description = str(live.get("description") or "") if live else "" + if live is not None and not labels: + labels = [str(x) for x in (live.get("labels") or [])] + attachment_urls = ( + [str(x) for x in (live.get("attachment_urls") or [])] if live else [] + ) + # The criteria a Done must satisfy: today's, and whatever this call writes. + # Evaluating only the proposed text would let a Done call drop a criterion. + descriptions = [d for d in (live_description, new_description) if d] + description = new_description if new_description is not None else live_description + pr_description = augment_description_with_attachments(description, attachment_urls) + + # (U) an unchecked box is the ticket's own admission it is not done + # (OMN-15030). Checked against every description in play. + for desc in descriptions or [""]: + unchecked_boxes = find_unchecked_acceptance_boxes(desc) + if unchecked_boxes: + preview = "; ".join(unchecked_boxes[:5]) + more = ( + f" (+{len(unchecked_boxes) - 5} more)" + if len(unchecked_boxes) > 5 + else "" + ) + return _refuse( + f"unchecked_acceptance_criteria: {len(unchecked_boxes)} unchecked " + f"box(es) remain in the ticket description: {preview}{more}. " + "Ticking them is not the remedy: a box is ticked only once a " + "bound PASS dod_verify receipt covers it (OMN-15030 / OMN-20368)." ) - - # (2b) unchecked acceptance-criteria checkbox gate (OMN-15030). Runs BEFORE - # every evidence path below and is not waivable by any of them — a merged - # PR, a PASS OCC receipt, or an exempt label all prove *some* evidence - # exists, none of them prove the ticket's own stated acceptance criteria - # were met. An unchecked `- [ ]` box in the ticket's current description - # is the ticket author's own admission of incompleteness; refusing here is - # the mechanical, judgment-free proxy for "does the shipped PR satisfy - # this ticket's DoD" that no presence-check below can express. - unchecked_boxes = find_unchecked_acceptance_boxes(description) - if unchecked_boxes: - preview = "; ".join(unchecked_boxes[:5]) - more = ( - f" (+{len(unchecked_boxes) - 5} more)" if len(unchecked_boxes) > 5 else "" - ) - return Decision( - False, - f"unchecked_acceptance_criteria: {len(unchecked_boxes)} unchecked " - f"box(es) remain in the ticket description: {preview}{more}. Check " - "every acceptance-criteria box (or remove/rewrite the ones that no " - "longer apply) before flipping Done — a merged PR or OCC receipt " - "does not waive this (OMN-15030 / OMN-13991).", - ) default_repo = os.environ.get("LINEAR_DONE_VERIFY_DEFAULT_REPO") or None - # (3-pre) durable evidence path C — deploy-readback marker (OMN-14792). A - # runtime-deploy ticket's DoD is a live readback (effects image rebuilt to - # dev-tip + a clean probe read off the deployed bytes), NOT a merged product - # PR: node_dod_verify structurally skips such tickets - # (reference_dod_verify_cannot_close_deploy_tickets) and they close via an - # operator deliberate-Done. Before this carve-out the guard false-blocked - # them (the OMN-14437 shape) because it treated every PR string in the body - # — a scratch/throwaway live-mint readback PR, and bare merge-chain-narrative - # numbers — as blocking DoD evidence. - # - # When a `deploy-readback-proven:` marker with real evidence is present, the - # merge check is SCOPED to DoD-*implementing* PRs only (verify_implementing: - # full-URL / resolvable bare #N, excluding scratch-annotated and - # unresolvable-narrative refs). If any implementing PR is unmerged the flip - # still BLOCKS — the marker is not a blanket bypass (the OMN-14641 lesson). - # Otherwise the live-readback attestation stands in for a merged PR. - if parse_deploy_readback_marker(description) is not None: + # (P) every cited / linked product PR must be merged. A further condition, + # never a sufficient one (OMN-20368). + merged_pr = False + if parse_deploy_readback_marker(pr_description) is not None: impl_result = verify_implementing( - description, labels, default_repo=default_repo, fetcher=pr_fetcher + pr_description, labels, default_repo=default_repo, fetcher=pr_fetcher ) if not impl_result.allowed: - return Decision(False, f"pr_not_merged\n{impl_result.reason}") - return Decision(True, "durable_evidence:deploy_readback_proven") - - # (3) durable evidence path A — merged product-PR citation. This runs BEFORE - # the close-if-done exemption carve-out (OMN-14641): the label was a blanket - # merge-check bypass, so a ticket carrying it could flip Done with its linked - # product PR still OPEN. verify() now blocks on any open cited product PR - # regardless of the label; the exemption is honored only below, when NO - # product PR is cited. - pr_result = verify( - description, - labels, - default_repo=default_repo, - fetcher=pr_fetcher, - ticket_id=ticket_id or None, - ) - if not pr_result.allowed: - # A cited PR is open / unmerged / unresolvable — the classic OMN-8375 - # "Done while PR still BLOCKED" shape (and the OMN-14582 label-bypass - # shape). Block outright — the close-if-done label does not waive this. - # - # OMN-15712: before blocking, check whether the ticket's own OCC DoD - # contract still considers each blocking PR load-bearing. A ticket - # that HAS OCC receipts (a real dod_verify contract instance) may have - # legitimately superseded a stale attachment (an explicit PASS - # `-superseded` receipt) or never cited it at all (a leftover - # attachment, e.g. an old release PR replaced by a later one) — in - # either case the stale PR's live state is not this ticket's evidence - # and must not block. A ticket with ZERO OCC receipts is unaffected — - # there is no contract data to reason "cited" from, so every blocking - # ref is treated exactly as before this feature existed. - if ticket_id: - lister = receipt_lister - if lister is None: - omni_home = resolve_omni_home() - if omni_home is not None: - _repo = occ_repo_path(omni_home) - lister = lambda tid: list_ticket_receipt_fields(_repo, tid) # noqa: E731 - if lister is not None: - receipts = lister(ticket_id) - if ( - receipts - ): # ticket HAS an OCC contract — citation-aware filtering applies - blocking = [ - s for s in pr_result.pr_statuses if classify_blocking(s) - ] - # OMN-15712 regression guard: the citation-awareness carve-out - # is a supersession/leftover-attachment concept, NOT a general - # "no receipt mentions it" bypass. An OPEN PR (or an - # unresolvable probe, i.e. ``error`` set) is potentially - # load-bearing in-flight work regardless of what the OCC - # contract has or hasn't cited yet — it ALWAYS blocks, exactly - # as the pre-OMN-15712 guard did. Only a CLOSED-unmerged ref - # is eligible for the uncited/superseded carve-out, because - # only a closed PR can be a *stale* attachment the contract - # legitimately moved past. - still_blocking = [ - s - for s in blocking - if s.error - or s.state == "OPEN" - or pr_citation_state(receipts, s.ref.number) - not in ("uncited", "superseded") - ] - if not still_blocking: - return Decision( - True, - "durable_evidence:blocking_refs_uncited_or_superseded_" - "by_occ_contract", - ) - return Decision( - False, - f"pr_not_merged\n{_format_blocking_lines(still_blocking)}", - ) - return Decision(False, f"pr_not_merged\n{pr_result.reason}") - if pr_result.reason == "all_prs_merged": - return Decision(True, "durable_evidence:all_prs_merged") - - # (2) explicit exemption label / close-if-done frontmatter. Covers - # decision-only tickets and epic ALL_CHILDREN_DONE roll-ups (which carry the - # label). Encoded explicitly — never inferred from ticket shape. Valid ONLY - # here, where no product PR is cited; an open cited PR already returned above - # (OMN-14641 — the label can no longer bypass an unmerged linked PR). - if is_exempt(description, labels): - return Decision(True, "carve_out:exempt_label") - - # pr_result.allowed with reason "no_pr_references" is NOT durable evidence on - # its own — this is the incident shape (Done, no PR cited). Fall through to - # the git-backed OCC receipt path; do NOT allow here. - - if not ticket_id: - return Decision( - False, - "no_ticket_id: cannot verify durable evidence for a Done-flip without " - "an issue id. Pass 'id' in the save_issue call.", + return _refuse(f"pr_not_merged\n{impl_result.reason}") + else: + pr_result = verify( + pr_description, + labels, + default_repo=default_repo, + fetcher=pr_fetcher, + ticket_id=ticket_id or None, ) - - # (4) durable evidence path B — the no-PR bar (OMN-13856): every labelled - # acceptance criterion bound in the OCC contract to a fresh, attested - # receipt, read off origin/dev. Reached only when no PR is cited: every - # cited PR was decided above. A self-written description line is not - # evidence here; only the change-control record is. - probe = occ_probe - if probe is None: - omni_home = resolve_omni_home() - if omni_home is None: - return Decision( - False, - "no_durable_evidence: OMNI_HOME is unset/invalid, so the local " - "onex_change_control clone cannot be resolved and no merged-PR " - "citation was found. Failing closed (design requirement 4 — never " - "fail-open on a fake-Done). Set OMNI_HOME, or cite the merged " - "implementing PR in the ticket description.", - ) - repo = occ_repo_path(omni_home) - clock = now or datetime.now(UTC) - probe = lambda tid, desc: evaluate_bound_evidence( # noqa: E731 - tid, desc, load_ticket_occ_evidence(repo, tid), clock - ) - - verdict = probe(ticket_id, description) - if verdict.passed: - return Decision(True, "durable_evidence:occ_bound_receipts") - + if pr_result.allowed: + merged_pr = pr_result.reason == "all_prs_merged" + else: + still_blocking = [s for s in pr_result.pr_statuses if classify_blocking(s)] + receipts: list[dict[str, str]] = [] + if ticket_id: + lister = receipt_lister + if lister is None: + workspace_root = resolve_omni_home() + if workspace_root is not None: + lister = _occ_lister(occ_repo_path(workspace_root)) + if lister is not None: + receipts = lister(ticket_id) + if receipts: + # OMN-15712: a CLOSED-unmerged attachment the contract never cited, + # or explicitly superseded, is not load-bearing. OPEN always blocks. + still_blocking = [ + s + for s in still_blocking + if s.error + or s.state == "OPEN" + or pr_citation_state(receipts, s.ref.number) + not in ("uncited", "superseded") + ] + if still_blocking: + return _refuse( + f"pr_not_merged\n{_format_blocking_lines(still_blocking)}" + ) + merged_pr = any(s.state == "MERGED" for s in pr_result.pr_statuses) + else: + return _refuse(f"pr_not_merged\n{pr_result.reason}") + + # (B) the bound-receipt bar. The one sufficient condition, on every path. + refused = _bar( + descriptions or [description], + merged_pr=merged_pr, + why=( + "a Done transition needs a passing definition-of-done check for " + "every acceptance criterion" + ), + ) + if refused is not None: + return refused return Decision( - False, - f"no_durable_evidence for {ticket_id}: no merged-PR citation in the " - f"description, and the no-PR bar is not met on {_OCC_REF} of the local " - f"onex_change_control clone: {verdict.detail}. Fail-closed (no fake " - "Done). Cite the merged implementing PR in the ticket description; or " - f"land contracts/{ticket_id}.yaml binding every labelled acceptance " - "criterion via `binds_ac`, each with a PASS receipt under " - f"{_RECEIPT_DIR_PREFIX}/{ticket_id}/ naming the ticket, the item, the " - "environment (target_identity or working_dir) and a run_timestamp " - "inside the freshness window, attested by a verifier other than its " - "runner; or apply an explicit close-if-done exemption for a legitimate " - "no-PR close.", + True, + "durable_evidence:occ_bound_receipts" + + (":all_prs_merged" if merged_pr else ""), ) @@ -718,14 +859,36 @@ def _load_stdin_call() -> dict[str, Any]: def main() -> int: - """Read a PreToolUse tool call on stdin; exit 0 (allow) or 2 (block).""" + """Read a PreToolUse tool call on stdin; exit 0 (allow) or 2 (block). + + OMN-20368: fail CLOSED. A guard that cannot decide refuses the Linear + write and says why; before this a runtime error in the guard was an allow. + """ call = _load_stdin_call() - decision = decide(call) + try: + decision = decide(call) + except ( + ArithmeticError, + AttributeError, + ImportError, + LookupError, + OSError, + RuntimeError, + TypeError, + ValueError, + subprocess.SubprocessError, + ) as exc: + decision = Decision( + False, + f"guard_error: the done-flip guard could not decide " + f"({type(exc).__name__}: {exc}). Refusing the Linear write rather " + "than letting it through unchecked (OMN-20368).", + ) if decision.allowed: return 0 payload = { "decision": "block", - "reason": f"[OMN-13856 done-flip durable-evidence gate] {decision.reason}", + "reason": f"[OMN-20368 done-flip bound-receipt gate] {decision.reason}", } sys.stderr.write(json.dumps(payload) + "\n") return 2 diff --git a/plugins/onex/hooks/lib/no_pr_bound_evidence.py b/plugins/onex/hooks/lib/no_pr_bound_evidence.py index 22fa32de20..20361b8b17 100644 --- a/plugins/onex/hooks/lib/no_pr_bound_evidence.py +++ b/plugins/onex/hooks/lib/no_pr_bound_evidence.py @@ -64,7 +64,10 @@ from __future__ import annotations +import hashlib +import os import re +import signal import subprocess # noqa: S404 - fixed-argv git invocations, no shell from dataclasses import dataclass, field from datetime import UTC, datetime, timedelta @@ -82,6 +85,10 @@ _RECEIPT_DIR_PREFIX = "drift/dod_receipts" _RETIREMENT_FIELD = "supersedes_ac_binding" _ENVIRONMENT_FIELDS = ("target_identity", "working_dir") +# OMN-20368: a receipt behind a MERGED product PR names the code it read by its +# commit, so the commit is that receipt's environment. The no-PR bar keeps the +# narrower pair above: a commit is not where a state readback was taken. +MERGED_PR_ENVIRONMENT_FIELDS = ("target_identity", "working_dir", "commit_sha") _GIT_FETCH_TIMEOUT_SECONDS = 20 _GIT_READ_TIMEOUT_SECONDS = 15 @@ -320,18 +327,99 @@ def _timestamp(value: object) -> datetime | None: return parsed if parsed.tzinfo is not None else None +def _entry_hash(contract: dict[str, Any], item_id: str) -> str | None: + """The canonical per-entry hash OCC binds receipts to, or None. + + Imported from omnibase_core (a runtime dependency of this plugin), never + reimplemented: occ-preflight and the Receipt Gate use the same function, so + a port here could disagree with them silently. None when it cannot be + computed, which the caller refuses. + """ + try: + from omnibase_core.validation.validator_receipt_gate import ( + compute_contract_entry_sha256, + ) + except ImportError: + return None + try: + return str(compute_contract_entry_sha256(contract, item_id)) + except (LookupError, TypeError, ValueError): + # ContractEntryNotFoundError is a LookupError: "cannot recompute", + # which the caller refuses. + return None + + +def _normalised_hash(value: str) -> str: + text = value.strip().lower() + return text if text.startswith("sha256:") else f"sha256:{text}" + + +def staleness_defect( + receipt: dict[str, Any], + *, + item: EvidenceItem, + contract: dict[str, Any], + contract_sha256: str, +) -> str | None: + """Why ``receipt`` was taken against a different contract than today's, or None. + + OMN-20368. A receipt is evidence about the contract it was run against. When + the contract entry it discharges has changed since, the receipt proves the + old entry, not the current one, and a Done resting on it is unearned. + + * ``contract_entry_sha256`` present: it must equal the per-entry hash of + the item in the contract on ``origin/dev`` today. + * otherwise ``contract_sha256`` (legacy, whole file) must equal the hash of + today's contract bytes. + * neither: the receipt names no contract version at all, so nothing ties + it to today's criteria. + """ + entry_hash = _str(receipt, "contract_entry_sha256") + if entry_hash: + current = _entry_hash(contract, item.item_id) + if current is None: + return ( + "per-entry contract hash cannot be recomputed at hook time " + "(omnibase_core.validation.validator_receipt_gate unavailable)" + ) + if _normalised_hash(entry_hash) != _normalised_hash(current): + return ( + f"is stale: contract entry {item.item_id} changed since the " + "receipt was taken (contract_entry_sha256 does not match the " + "contract on origin/dev); re-run dod_verify and land a new receipt" + ) + return None + whole = _str(receipt, "contract_sha256") + if whole: + if not contract_sha256 or _normalised_hash(whole) != _normalised_hash( + contract_sha256 + ): + return ( + "is stale: the contract changed since the receipt was taken " + "(contract_sha256 does not match the contract on origin/dev); " + "re-run dod_verify and land a new receipt" + ) + return None + return "names no contract version (no contract_entry_sha256 or contract_sha256)" + + def receipt_defect( receipt: dict[str, Any], *, ticket_id: str, item: EvidenceItem, now: datetime, - max_age: timedelta = NO_PR_RECEIPT_MAX_AGE, + max_age: timedelta | None = NO_PR_RECEIPT_MAX_AGE, + environment_fields: tuple[str, ...] = _ENVIRONMENT_FIELDS, + contract: dict[str, Any] | None = None, + contract_sha256: str = "", ) -> str | None: """Why ``receipt`` does not discharge ``item`` for ``ticket_id``, or None. - None means the receipt names the subject, the environment and a fresh read - time, and is an independently attested PASS. Pure function. + None means the receipt names the subject, the environment and a read time + (fresh, when ``max_age`` is set), is an independently attested PASS, and, + when ``contract`` is given, was taken against today's contract entry + (OMN-20368). Pure function apart from the omnibase_core hash import. """ if _str(receipt, "status").upper() != "PASS": return "not a PASS" @@ -352,19 +440,23 @@ def receipt_defect( return "is self-attested or names no runner/verifier" if not _str(receipt, "probe_stdout"): return "carries no probe_stdout (no observation)" - if not any(_str(receipt, key) for key in _ENVIRONMENT_FIELDS): - return "names no environment (target_identity or working_dir)" + if not any(_str(receipt, key) for key in environment_fields): + return f"names no environment ({' or '.join(environment_fields)})" read_at = _timestamp(receipt.get("run_timestamp")) if read_at is None: return "has no timezone-aware run_timestamp (no read time)" now_utc = now.astimezone(UTC) if read_at - now_utc > _MAX_FUTURE_SKEW: return f"read time {read_at.isoformat()} is in the future" - if now_utc - read_at > max_age: + if max_age is not None and now_utc - read_at > max_age: return ( f"read time {read_at.isoformat()} is older than the " f"{max_age.days}-day freshness window" ) + if contract is not None: + return staleness_defect( + receipt, item=item, contract=contract, contract_sha256=contract_sha256 + ) return None @@ -383,6 +475,9 @@ class OccTicketEvidence: contract: dict[str, Any] | None receipts: list[dict[str, Any]] = field(default_factory=list) error: str = "" + # OMN-20368: ``sha256:`` of the contract's bytes on the ref, the value + # a legacy whole-file ``contract_sha256`` receipt binding is compared to. + contract_sha256: str = "" def evaluate_bound_evidence( @@ -391,9 +486,18 @@ def evaluate_bound_evidence( evidence: OccTicketEvidence, now: datetime, *, - max_age: timedelta = NO_PR_RECEIPT_MAX_AGE, + max_age: timedelta | None = NO_PR_RECEIPT_MAX_AGE, + environment_fields: tuple[str, ...] = _ENVIRONMENT_FIELDS, + check_staleness: bool = True, ) -> BoundEvidenceVerdict: - """Apply the no-PR bar (module docstring, steps 1-4). Pure function.""" + """Apply the bound-receipt bar (module docstring, steps 1-4). Pure function. + + OMN-20368: this is now the bar for EVERY Done flip, not only the no-PR one. + ``check_staleness`` refuses a receipt taken against a contract entry that + has changed since; ``max_age`` (None for no window) and + ``environment_fields`` let the merged-PR path judge a receipt about merged + code by its commit rather than by a state-read window. + """ contract_path = f"{_CONTRACT_DIR}/{ticket_id}.yaml" if evidence.contract is None: why = f" ({evidence.error})" if evidence.error else "" @@ -445,7 +549,14 @@ def evaluate_bound_evidence( continue for receipt in candidates: defect = receipt_defect( - receipt, ticket_id=ticket_id, item=item, now=now, max_age=max_age + receipt, + ticket_id=ticket_id, + item=item, + now=now, + max_age=max_age, + environment_fields=environment_fields, + contract=evidence.contract if check_staleness else None, + contract_sha256=evidence.contract_sha256, ) if defect is None: discharged_by = item.item_id @@ -489,6 +600,38 @@ def _git( return None +def bounded_fetch(repo: Path, timeout: float = _GIT_FETCH_TIMEOUT_SECONDS) -> None: + """``git fetch --quiet origin dev`` in ``repo``, over within ``timeout`` seconds. + + OMN-20368. A fetch run with captured output waits for EOF on its pipes, + and the ssh or https helper git spawns keeps them open after git itself is + killed, so ``subprocess.run(..., capture_output=True, timeout=...)`` can + hang well past its timeout. The hook harness then times the guard out and + lets the Linear write through, which is how two Done-class writes passed + unchecked during the live probe on a loaded host. Here nothing is captured, + the fetch runs in its own process group, and the whole group is killed at + the deadline. Any failure is ignored: the caller reads the last-known ref. + """ + try: + proc = subprocess.Popen( + ["git", "-C", str(repo), "fetch", "--quiet", "origin", "dev"], + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + start_new_session=True, + ) + except OSError: + return + try: + proc.wait(timeout=timeout) + except subprocess.TimeoutExpired: + try: + os.killpg(proc.pid, signal.SIGKILL) + except OSError: + pass + proc.wait() + + def load_ticket_occ_evidence( occ_repo: Path, ticket_id: str, *, ref: str = "origin/dev", fetch: bool = True ) -> OccTicketEvidence: @@ -505,9 +648,7 @@ def load_ticket_occ_evidence( return OccTicketEvidence(None, error=f"no OCC clone at {occ_repo}") if fetch: # Best effort: a failed fetch still reads the last-known ref. - _git( - ["fetch", "--quiet", "origin", "dev"], occ_repo, _GIT_FETCH_TIMEOUT_SECONDS - ) + bounded_fetch(occ_repo) shown = _git( ["show", f"{ref}:{_CONTRACT_DIR}/{ticket_id}.yaml"], @@ -522,6 +663,9 @@ def load_ticket_occ_evidence( return OccTicketEvidence(None, error=f"contract YAML unreadable: {exc}") if not isinstance(contract, dict): return OccTicketEvidence(None, error="contract is not a mapping") + contract_sha256 = ( + f"sha256:{hashlib.sha256(shown.stdout.encode('utf-8')).hexdigest()}" + ) receipts: list[dict[str, Any]] = [] listing = _git( @@ -552,19 +696,22 @@ def load_ticket_occ_evidence( continue if isinstance(parsed, dict): receipts.append(parsed) - return OccTicketEvidence(contract, receipts) + return OccTicketEvidence(contract, receipts, contract_sha256=contract_sha256) __all__ = [ + "MERGED_PR_ENVIRONMENT_FIELDS", "NO_PR_RECEIPT_MAX_AGE", "BoundEvidenceVerdict", "EvidenceItem", "OccTicketEvidence", "acceptance_criteria_items", + "bounded_fetch", "canonical_ac_label", "evaluate_bound_evidence", "evidence_items", "live_acceptance_criteria_items", "load_ticket_occ_evidence", "receipt_defect", + "staleness_defect", ] diff --git a/plugins/onex/hooks/scripts/pre_tool_use_done_flip_guard.sh b/plugins/onex/hooks/scripts/pre_tool_use_done_flip_guard.sh index 4824f023e3..d246e738e4 100755 --- a/plugins/onex/hooks/scripts/pre_tool_use_done_flip_guard.sh +++ b/plugins/onex/hooks/scripts/pre_tool_use_done_flip_guard.sh @@ -2,15 +2,20 @@ # SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. # SPDX-License-Identifier: MIT # -# Done-flip durable-evidence guard [OMN-13856] — L1 of the layered Done-flip -# gate. PreToolUse hook on mcp__linear-server__{save,update}_issue. +# Done-flip bound-receipt guard [OMN-13856, OMN-20368]. PreToolUse hook on +# mcp__linear-server__{save,update}_issue. # -# Restored as the MINIMAL Option A carve-out into the otherwise-empty -# OMN-13244 measurement baseline: this is the SINGLE hook re-registered, guarding -# the single most dangerous mutation class (a fabricated Done). The guard MERGES -# the merged-PR (linear_done_verify) and receipt-PASS (dod_completion) semantics -# into one fail-closed decision and mechanizes the receipt check on the -# DETERMINISTIC LOCAL dod_verify path (no Kafka). See done_flip_guard.py. +# A Linear ticket reaches Done only when its OCC contract binds every +# acceptance criterion to a PASS receipt taken against the current contract +# entry, and an acceptance box is ticked only under the same receipt. The +# decision lives in done_flip_guard.py. +# +# FAIL CLOSED (OMN-20368). Before this ticket the wrapper let a call through +# when the cwd was not an omninode repo, when mode.sh said lite, when the hooks +# mask was set, when the decision library was missing, and when the library +# crashed. A Linear ticket is global state, so none of those says anything +# about whether a Done is earned; each one is now a refusal or is gone. There +# is no skip token and no environment bypass. # # Exit codes: # 0 — allow the tool call @@ -19,66 +24,52 @@ set -eo pipefail # OMN-19381: named so its refusal row names it. This guard does not source -# error-guard.sh (its EXIT trap would rewrite this guard's bare `exit 2`), so it sources -# the refusal seam on its own, after common.sh. +# error-guard.sh (its EXIT trap would rewrite this guard's bare `exit 2`), so it +# sources the refusal seam on its own, after common.sh. _OMNICLAUDE_HOOK_NAME="${BASH_SOURCE[0]##*/}" -_OMNICLAUDE_CALLER_CWD="${CLAUDE_PROJECT_DIR:-$PWD}" -# shellcheck source=../lib/repo_guard.sh # OMN-20109: this script's directory, resolved once without a dirname exec. _ONEX_HOOK_SELF_DIR="${BASH_SOURCE[0]%/*}"; [[ "${BASH_SOURCE[0]}" == */* ]] || _ONEX_HOOK_SELF_DIR=.; [[ -n "$_ONEX_HOOK_SELF_DIR" ]] || _ONEX_HOOK_SELF_DIR=/ -. "${_ONEX_HOOK_SELF_DIR}/../lib/repo_guard.sh" 2>/dev/null || true -if declare -F is_omninode_repo >/dev/null 2>&1; then - CLAUDE_PROJECT_DIR="${CLAUDE_PROJECT_DIR:-$_OMNICLAUDE_CALLER_CWD}" \ - is_omninode_repo || { - _OMNICLAUDE_PASSTHROUGH=$(cat) - echo "$_OMNICLAUDE_PASSTHROUGH" - trap - EXIT 2>/dev/null || true - exit 0 - } -fi - -# Lite mode guard [OMN-5398] _SCRIPT_DIR="$(cd "${_ONEX_HOOK_SELF_DIR}" && pwd)" -_MODE_SH="${_SCRIPT_DIR}/../../lib/mode.sh" -if [[ -f "$_MODE_SH" ]]; then - source "$_MODE_SH" - [[ "$(omniclaude_mode)" == "lite" ]] && exit 0 -fi - PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-$(cd "${_SCRIPT_DIR}/../.." && pwd)}" LIB_PY="${PLUGIN_ROOT}/hooks/lib/done_flip_guard.py" +unset _SCRIPT_DIR + +_done_flip_refuse() { + # $1: reason. Refuse the Linear write, loudly, in the hook's JSON shape. + printf '{"decision": "block", "reason": "[OMN-20368 done-flip bound-receipt gate] %s"}\n' "$1" >&2 + exit 2 +} -# common.sh provides PYTHON_CMD resolution and shared helpers used by all hooks -# that invoke Python. Sourced here to satisfy the hooks-source-common invariant. +# The payload is read once, so the refusal recorder can read the lane from it +# and the decision core gets the same bytes on its stdin (OMN-19381). +_OMNICLAUDE_HOOK_PAYLOAD="$(cat)" + +# common.sh provides PYTHON_CMD resolution. A failure to source it is a +# refusal, not an allow: without an interpreter nothing can be decided. # shellcheck source=/dev/null -source "${PLUGIN_ROOT}/hooks/scripts/common.sh" +source "${PLUGIN_ROOT}/hooks/scripts/common.sh" \ + || _done_flip_refuse "guard_error: common.sh could not be sourced, so no interpreter is resolved" # OMN-19381: hook_record_refusal, without error-guard.sh's EXIT trap. # shellcheck source=../lib/hook_refusal.sh source "${PLUGIN_ROOT}/hooks/lib/hook_refusal.sh" 2>/dev/null || true -onex_hook_gate DONE_FLIP_GUARD || exit 0 -unset _SCRIPT_DIR _MODE_SH -if [[ ! -f "$LIB_PY" ]]; then - # Library missing — fail open so we never block on our own bug. - cat >/dev/null - exit 0 -fi +[[ -f "$LIB_PY" ]] || _done_flip_refuse "guard_error: decision library ${LIB_PY} is missing" + +PYTHON_BIN="${PYTHON_CMD:-}" +[[ -n "$PYTHON_BIN" ]] || _done_flip_refuse "guard_error: no Python interpreter resolved for the guard" -PYTHON_BIN="${PYTHON_CMD:-python3}" -# Only exit code 2 (blocking decision) should propagate. Any other non-zero -# exit is a Python runtime error in the hook itself — fail open to avoid -# blocking legitimate tool calls on a hook bug (never blocks on our own defect). -# OMN-19381: the payload is read here, once, so the refusal recorder can -# read the lane from it; the decision core gets the same bytes on its stdin. -_OMNICLAUDE_HOOK_PAYLOAD="$(cat)" set +e "$PYTHON_BIN" "$LIB_PY" <<<"$_OMNICLAUDE_HOOK_PAYLOAD" rc=$? set -e +if [[ "$rc" -eq 0 ]]; then + exit 0 +fi if [[ "$rc" -eq 2 ]]; then # OMN-18946: see hook_record_refusal in error-guard.sh. - hook_record_refusal "done flip refused without durable evidence" "a Done transition was refused by the durable-evidence guard" 2>/dev/null || true + hook_record_refusal "done flip refused without a bound dod receipt" "a Done transition or acceptance-box tick was refused by the bound-receipt guard" 2>/dev/null || true exit 2 fi -exit 0 +# Any other exit is the guard failing to decide. That refuses (OMN-20368). +_done_flip_refuse "guard_error: done_flip_guard.py exited ${rc} without a decision" diff --git a/scripts/public_skill_hygiene_ratchet.yaml b/scripts/public_skill_hygiene_ratchet.yaml index 340b847a82..ba6922c7e5 100644 --- a/scripts/public_skill_hygiene_ratchet.yaml +++ b/scripts/public_skill_hygiene_ratchet.yaml @@ -19,8 +19,8 @@ tree_surface_budgets: host-nick: 103 # public-skill-ok: detection-class name, not prose machine-path: 61 # public-skill-ok: detection-class name, not prose - memory-cite: 37 # public-skill-ok: detection-class name, not prose - omni_home-prose: 438 # public-skill-ok: detection-class name, not prose + memory-cite: 36 # public-skill-ok: detection-class name, not prose + omni_home-prose: 432 # public-skill-ok: detection-class name, not prose operator-email: 31 # public-skill-ok: detection-class name, not prose operator-name: 173 # public-skill-ok: detection-class name, not prose person-name: 2 # public-skill-ok: detection-class name, not prose diff --git a/tests/hooks/test_done_flip_guard.py b/tests/hooks/test_done_flip_guard.py index 7e02ecdb07..5a826b0c6a 100644 --- a/tests/hooks/test_done_flip_guard.py +++ b/tests/hooks/test_done_flip_guard.py @@ -56,6 +56,13 @@ def _load_guard() -> Any: guard = _load_guard() import no_pr_bound_evidence as nbe # noqa: E402 (sibling import needs sys.path) + +@pytest.fixture(autouse=True) +def _hermetic_linear(monkeypatch: pytest.MonkeyPatch) -> None: + """No test reaches Linear: the default fetcher reads an empty key as 'no live data'.""" + monkeypatch.setenv("LINEAR_API_KEY", "") + + # --------------------------------------------------------------------------- # # Stub factories # --------------------------------------------------------------------------- # @@ -139,7 +146,7 @@ def test_cancel_state_allows() -> None: assert d.reason == "carve_out:cancel_state" -def test_exempt_label_allows() -> None: +def test_exempt_label_alone_no_longer_closes_omn_20368() -> None: call = { "tool_name": "mcp__linear-server__save_issue", "tool_input": { @@ -149,9 +156,12 @@ def test_exempt_label_allows() -> None: "labels": ["close-if-done"], }, } - d = guard.decide(call, occ_probe=_never_called_probe) - assert d.allowed - assert d.reason == "carve_out:exempt_label" + d = guard.decide(call, occ_probe=_no_receipt_probe) + assert not d.allowed + assert "no_bound_dod_receipt" in d.reason + d = guard.decide(call, occ_probe=_receipt_probe) + assert d.allowed, d.reason + assert d.reason == "durable_evidence:occ_bound_receipts" # --------------------------------------------------------------------------- # @@ -223,7 +233,7 @@ def test_close_if_done_label_allows_merged_linked_attachment_pr() -> None: } d = guard.decide( call, - occ_probe=_never_called_probe, + occ_probe=_receipt_probe, pr_fetcher=_merged_fetcher, linear_fetcher=lambda _t: { "description": "", @@ -232,7 +242,7 @@ def test_close_if_done_label_allows_merged_linked_attachment_pr() -> None: }, ) assert d.allowed - assert d.reason == "durable_evidence:all_prs_merged" + assert d.reason == "durable_evidence:occ_bound_receipts:all_prs_merged" # --------------------------------------------------------------------------- # @@ -262,7 +272,7 @@ def test_incident_backlog_to_done_no_evidence_is_blocked() -> None: linear_fetcher=_no_linear, ) assert not d.allowed - assert "no_durable_evidence" in d.reason + assert "no_bound_dod_receipt" in d.reason def test_incident_status_only_update_no_evidence_is_blocked() -> None: @@ -337,9 +347,9 @@ def test_legit_merged_pr_citation_is_allowed() -> None: ), }, } - d = guard.decide(call, occ_probe=_never_called_probe, pr_fetcher=_merged_fetcher) + d = guard.decide(call, occ_probe=_receipt_probe, pr_fetcher=_merged_fetcher) assert d.allowed - assert d.reason == "durable_evidence:all_prs_merged" + assert d.reason == "durable_evidence:occ_bound_receipts:all_prs_merged" def test_legit_occ_receipt_is_allowed() -> None: @@ -454,9 +464,9 @@ def test_all_boxes_checked_is_not_blocked_by_the_checkbox_gate() -> None: ), }, } - d = guard.decide(call, occ_probe=_never_called_probe, pr_fetcher=_merged_fetcher) + d = guard.decide(call, occ_probe=_receipt_probe, pr_fetcher=_merged_fetcher) assert d.allowed - assert d.reason == "durable_evidence:all_prs_merged" + assert d.reason == "durable_evidence:occ_bound_receipts:all_prs_merged" def test_unchecked_box_blocks_even_with_occ_receipt_evidence() -> None: @@ -553,12 +563,12 @@ def test_deploy_readback_ticket_with_scratch_and_narrative_is_allowed() -> None: } d = guard.decide( call, - occ_probe=_never_called_probe, + occ_probe=_receipt_probe, pr_fetcher=_never_called_fetcher, linear_fetcher=_no_linear, ) assert d.allowed - assert d.reason == "durable_evidence:deploy_readback_proven" + assert d.reason == "durable_evidence:occ_bound_receipts" def test_deploy_readback_status_only_update_is_allowed() -> None: @@ -569,7 +579,7 @@ def test_deploy_readback_status_only_update_is_allowed() -> None: } d = guard.decide( call, - occ_probe=_never_called_probe, + occ_probe=_receipt_probe, pr_fetcher=_never_called_fetcher, linear_fetcher=lambda _t: { "description": _OMN_14437_DESCRIPTION, @@ -577,7 +587,7 @@ def test_deploy_readback_status_only_update_is_allowed() -> None: }, ) assert d.allowed - assert d.reason == "durable_evidence:deploy_readback_proven" + assert d.reason == "durable_evidence:occ_bound_receipts" def test_deploy_readback_marker_does_not_bypass_open_implementing_pr() -> None: @@ -609,9 +619,9 @@ def test_deploy_readback_marker_allows_when_implementing_pr_merged() -> None: "tool_name": "mcp__linear-server__save_issue", "tool_input": {"id": "OMN-14998", "state": "Done", "description": desc}, } - d = guard.decide(call, occ_probe=_never_called_probe, pr_fetcher=_merged_fetcher) + d = guard.decide(call, occ_probe=_receipt_probe, pr_fetcher=_merged_fetcher) assert d.allowed - assert d.reason == "durable_evidence:deploy_readback_proven" + assert d.reason == "durable_evidence:occ_bound_receipts" def test_empty_deploy_readback_marker_is_not_accepted() -> None: @@ -629,7 +639,7 @@ def test_empty_deploy_readback_marker_is_not_accepted() -> None: linear_fetcher=_no_linear, ) assert not d.allowed - assert "no_durable_evidence" in d.reason + assert "no_bound_dod_receipt" in d.reason def test_code_ticket_open_implementing_pr_blocked_without_marker() -> None: @@ -773,6 +783,22 @@ def _make_occ_clone_with_dev_receipt( commit_sha: "abc1234" """ + +def _entry_sha(contract_text: str, item: str) -> str: + import yaml + from omnibase_core.validation.validator_receipt_gate import ( + compute_contract_entry_sha256, + ) + + return str(compute_contract_entry_sha256(yaml.safe_load(contract_text), item)) + + +# OMN-20368: a receipt names the contract entry it was taken against. +_BOUND_RECEIPT = ( + _BOUND_RECEIPT + + f'contract_entry_sha256: "{_entry_sha(_BOUND_CONTRACT, "dod-001")}"\n' +) + _BOUND_DESCRIPTION = "## Acceptance criteria\n\n- AC1: no dirty worktree remains\n" _BOUND_NOW = datetime(2026, 9, 25, 15, 0, tzinfo=UTC) @@ -841,7 +867,7 @@ def test_occ_bare_pass_receipt_no_longer_closes_a_no_pr_ticket(tmp_path: Path) - } d = guard.decide(call, occ_probe=_bound_probe(clone), linear_fetcher=_no_linear) assert not d.allowed - assert "no_durable_evidence" in d.reason + assert "no_bound_dod_receipt" in d.reason assert "no OCC contract" in d.reason @@ -940,7 +966,7 @@ def test_uncited_stale_attachment_is_allowed_omn_15539_shape() -> None: } d = guard.decide( call, - occ_probe=_never_called_probe, + occ_probe=_receipt_probe, pr_fetcher=_closed_fetcher_for(1533), linear_fetcher=lambda _t: { "description": "Deterministic implementation contract shipped.", @@ -954,10 +980,7 @@ def test_uncited_stale_attachment_is_allowed_omn_15539_shape() -> None: receipt_lister=lambda _t: receipts, ) assert d.allowed - assert ( - d.reason - == "durable_evidence:blocking_refs_uncited_or_superseded_by_occ_contract" - ) + assert d.reason.startswith("durable_evidence:occ_bound_receipts"), d.reason def test_superseded_stale_attachment_is_allowed_omn_15422_shape() -> None: @@ -976,7 +999,7 @@ def test_superseded_stale_attachment_is_allowed_omn_15422_shape() -> None: } d = guard.decide( call, - occ_probe=_never_called_probe, + occ_probe=_receipt_probe, pr_fetcher=_closed_fetcher_for(2596), linear_fetcher=lambda _t: { "description": "Fixture provenance and sanitization documented.", @@ -989,10 +1012,7 @@ def test_superseded_stale_attachment_is_allowed_omn_15422_shape() -> None: receipt_lister=lambda _t: receipts, ) assert d.allowed - assert ( - d.reason - == "durable_evidence:blocking_refs_uncited_or_superseded_by_occ_contract" - ) + assert d.reason.startswith("durable_evidence:occ_bound_receipts"), d.reason def test_adversarial_active_unmerged_pr_still_blocks_despite_receipts() -> None: @@ -1222,7 +1242,7 @@ def _errors_on_unresolvable(ref: Any) -> Any: d = guard.decide( call, - occ_probe=_never_called_probe, + occ_probe=_receipt_probe, pr_fetcher=_errors_on_unresolvable, linear_fetcher=lambda _t: { "description": ( @@ -1235,4 +1255,4 @@ def _errors_on_unresolvable(ref: Any) -> Any: receipt_lister=lambda _t: [], ) assert d.allowed, d.reason - assert d.reason == "durable_evidence:all_prs_merged" + assert d.reason == "durable_evidence:occ_bound_receipts:all_prs_merged" diff --git a/tests/hooks/test_done_flip_guard_false_positives.py b/tests/hooks/test_done_flip_guard_false_positives.py index 9218b279f8..6080eec29a 100644 --- a/tests/hooks/test_done_flip_guard_false_positives.py +++ b/tests/hooks/test_done_flip_guard_false_positives.py @@ -70,6 +70,12 @@ def _load_guard() -> Any: _NOW = datetime(2026, 9, 25, 15, 0, tzinfo=UTC) +@pytest.fixture(autouse=True) +def _hermetic_linear(monkeypatch: pytest.MonkeyPatch) -> None: + """No test reaches Linear: the default fetcher reads an empty key as 'no live data'.""" + monkeypatch.setenv("LINEAR_API_KEY", "") + + def _call(ticket_id: str, description: str) -> dict[str, Any]: return { "tool_name": "mcp__linear-server__save_issue", @@ -81,6 +87,11 @@ def _no_receipt_probe(_ticket_id: str, _description: str) -> Any: return nbe.BoundEvidenceVerdict(False, "no OCC contract on origin/dev") +def _pass_probe(_ticket_id: str, _description: str) -> Any: + """Every criterion bound (OMN-20368): these tests are about PR resolution.""" + return nbe.BoundEvidenceVerdict(True, "bound: AC1<-dod-001") + + def _no_receipts(_ticket_id: str) -> list[dict[str, str]]: return [] @@ -152,6 +163,18 @@ def fetch(ref: Any) -> Any: } +def _entry_sha(item: str) -> str | None: + from omnibase_core.validation.validator_receipt_gate import ( + ContractEntryNotFoundError, + compute_contract_entry_sha256, + ) + + try: + return str(compute_contract_entry_sha256(_CONTRACT, item)) + except ContractEntryNotFoundError: + return None + + def _receipt(item: str, **overrides: Any) -> dict[str, Any]: receipt: dict[str, Any] = { "schema_version": "1.0.0", @@ -165,6 +188,8 @@ def _receipt(item: str, **overrides: Any) -> dict[str, Any]: "target_identity": "host:operator-mac/worktrees", "probe_stdout": "8 of 8 flagged worktrees absent\n", "commit_sha": "abc1234", + # OMN-20368: the receipt names the contract entry it was taken against. + "contract_entry_sha256": _entry_sha(item), } receipt.update(overrides) return {k: v for k, v in receipt.items() if v is not None} @@ -193,7 +218,7 @@ def test_a_bare_live_state_line_no_longer_closes() -> None: now=_NOW, ) assert not d.allowed - assert "no_durable_evidence" in d.reason + assert "no_bound_dod_receipt" in d.reason assert not hasattr(ldv, "parse_live_state_marker") @@ -314,7 +339,7 @@ def test_a_control_defective_receipt_is_refused( now=_NOW, ) assert not d.allowed, why - assert "no_durable_evidence" in d.reason + assert "no_bound_dod_receipt" in d.reason assert expect in d.reason, (why, d.reason) assert "DOD2" in d.reason @@ -449,12 +474,12 @@ def _omn_14642_table( def test_b_closed_pr_superseded_by_named_merged_successor_is_allowed() -> None: d = guard.decide( _call("OMN-14642", _OMN_14642_ATTACHED), - occ_probe=_no_receipt_probe, + occ_probe=_pass_probe, pr_fetcher=_table_fetcher(_omn_14642_table()), receipt_lister=_no_receipts, ) assert d.allowed, d.reason - assert d.reason == "durable_evidence:all_prs_merged" + assert d.reason == "durable_evidence:occ_bound_receipts:all_prs_merged" def test_b_verify_records_the_proven_successor() -> None: @@ -575,7 +600,7 @@ def _c_table( def test_c_commit_anchored_narrative_ref_resolves_to_its_merged_pr() -> None: d = guard.decide( _call("OMN-14642", _NARRATIVE + _CITED), - occ_probe=_no_receipt_probe, + occ_probe=_pass_probe, pr_fetcher=_table_fetcher(_c_table()), receipt_lister=_no_receipts, ) diff --git a/tests/hooks/test_done_flip_guard_requires_bound_receipt.py b/tests/hooks/test_done_flip_guard_requires_bound_receipt.py new file mode 100644 index 0000000000..2245610f59 --- /dev/null +++ b/tests/hooks/test_done_flip_guard_requires_bound_receipt.py @@ -0,0 +1,689 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""A Linear Done needs a passing definition-of-done check [OMN-20368]. + +On 2026-10-02 OMN-20180, OMN-20181 and OMN-20182 reached Done after dod_verify +refused them with NO_ACCEPTANCE_CHECKS. The guard admitted them on its merged-PR +path once the lane had ticked the acceptance boxes itself: their contracts bind +no criterion, so nothing checked any criterion at all. + +These tests pin the replacement contract: + +* the ONE sufficient condition for Done is the bound-receipt bar: the OCC + contract on ``origin/dev`` binds every labelled criterion through + ``binds_ac``, each discharged by a PASS receipt taken against the current + contract entry; +* a merged PR, ticked boxes, a deploy-readback marker and an exemption label + are further conditions or nothing, never substitutes; +* an edit that ticks an acceptance box needs the same receipt; +* the guard fails closed: on its own error, in any cwd, in lite mode, and + under any hooks mask. + +The OCC side is a real local git repository (no network); Linear and GitHub +are injected stubs. +""" + +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import sys +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +import pytest +import yaml +from omnibase_core.validation.validator_receipt_gate import ( + compute_contract_entry_sha256, +) +from omnibase_core.validators.no_unguarded_git_subprocess import ( + scrub_git_location_env, +) + +pytestmark = pytest.mark.unit + +_PLUGIN_ROOT = Path(__file__).parent.parent.parent / "plugins" / "onex" +_LIB_DIR = _PLUGIN_ROOT / "hooks" / "lib" +_SCRIPT = _PLUGIN_ROOT / "hooks" / "scripts" / "pre_tool_use_done_flip_guard.sh" + + +def _load_guard() -> Any: + sys.path.insert(0, str(_LIB_DIR)) + spec = importlib.util.spec_from_file_location( + "done_flip_guard", _LIB_DIR / "done_flip_guard.py" + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules["done_flip_guard"] = module + spec.loader.exec_module(module) + return module + + +guard = _load_guard() +# Sibling modules, importable once _load_guard() put lib/ on sys.path. +ldv: Any = importlib.import_module("linear_done_verify") +nbe: Any = importlib.import_module("no_pr_bound_evidence") + +_TICKET = "OMN-20180" +_NOW = datetime(2026, 10, 2, 13, 0, tzinfo=UTC) +_PR_URL = "https://github.com/OmniNode-ai/omnimarket/pull/3144" + + +@pytest.fixture(autouse=True) +def _hermetic_linear(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LINEAR_API_KEY", "") + + +# --------------------------------------------------------------------------- # +# Fixtures: a real OCC clone whose origin/dev carries the contract + receipts +# --------------------------------------------------------------------------- # + + +def _git(cwd: Path, *args: str) -> None: + subprocess.run( + ["git", "-C", str(cwd), *args], + check=True, + capture_output=True, + text=True, + env=scrub_git_location_env(os.environ), + ) + + +def _occ_clone(tmp_path: Path, contract: str, receipts: dict[str, str]) -> Path: + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "--bare", str(origin)], + check=True, + capture_output=True, + env=scrub_git_location_env(os.environ), + ) + clone = tmp_path / "onex_change_control" + subprocess.run( + ["git", "clone", str(origin), str(clone)], + check=True, + capture_output=True, + env=scrub_git_location_env(os.environ), + ) + _git(clone, "config", "user.email", "t@example.com") + _git(clone, "config", "user.name", "t") + _git(clone, "checkout", "-b", "dev") + (clone / "contracts").mkdir() + (clone / "contracts" / f"{_TICKET}.yaml").write_text(contract, encoding="utf-8") + for item, text in receipts.items(): + rp = clone / "drift" / "dod_receipts" / _TICKET / item + rp.mkdir(parents=True) + (rp / "command.yaml").write_text(text, encoding="utf-8") + _git(clone, "add", "-A") + _git(clone, "commit", "-m", "contract and receipts") + _git(clone, "push", "origin", "dev") + return clone + + +# The OMN-20180 contract as it is on origin/dev: two evidence items, PASS +# receipts for both, and no `binds_ac` anywhere. dod_verify calls that +# NO_ACCEPTANCE_CHECKS. +_UNBOUND_CONTRACT = """\ +schema_version: "1.0.0" +ticket_id: "OMN-20180" +dod_evidence: + - id: "dod-pr-3144" + checks: + - check_type: "command" + check_value: "grep -c PROTECTED_SURFACE scripts/ci/test_selection_models.py" + - id: "dod-pr-3144-ci" + checks: + - check_type: "command" + check_value: "gh pr view 3144 --repo OmniNode-ai/omnimarket --json files" +""" + +_BOUND_CONTRACT = """\ +schema_version: "1.0.0" +ticket_id: "OMN-20180" +dod_evidence: + - id: "dod-ac1" + binds_ac: ["AC1"] + checks: + - check_type: "command" + check_value: "uv run pytest tests/unit/scripts/test_detect_test_paths.py -k protected" + - id: "dod-ac2" + binds_ac: ["AC2"] + checks: + - check_type: "command" + check_value: "uv run pytest tests/unit/scripts/test_detect_test_paths.py -k unmapped" +""" + + +def _receipt( + item: str, + contract: str, + *, + entry_hash: str | None = "auto", + whole_hash: str | None = None, + status: str = "PASS", + run_timestamp: str = "2026-09-30T22:05:00Z", +) -> str: + fields: dict[str, Any] = { + "schema_version": "1.0.0", + "ticket_id": _TICKET, + "evidence_item_id": item, + "check_type": "command", + "status": status, + "run_timestamp": run_timestamp, + "runner": "lane-a", + "verifier": "lane-b", + "commit_sha": "0ec0302f795b36adabd86bf7d01fd10130384857", + "probe_stdout": "66 passed\n", + } + if entry_hash == "auto": + fields["contract_entry_sha256"] = compute_contract_entry_sha256( + yaml.safe_load(contract), item + ) + elif entry_hash is not None: + fields["contract_entry_sha256"] = entry_hash + if whole_hash is not None: + fields["contract_sha256"] = whole_hash + return yaml.safe_dump(fields, sort_keys=False) + + +_TICKED_DESCRIPTION = f"""\ +Smart test selection skips the delegation tests. + +## Acceptance criteria + +- [X] AC1: a delegation-surface change selects the full suite -- falsifier: pytest -k protected +- [X] AC2: an unmapped module escalates to the full suite -- falsifier: pytest -k unmapped + +Implemented in {_PR_URL} +""" + +_UNTICKED_DESCRIPTION = _TICKED_DESCRIPTION.replace("- [X]", "- [ ]") + + +def _merged(ref: Any) -> Any: + return ldv.PRStatus(ref=ref, state="MERGED", merge_state="CLEAN") + + +def _done(description: str | None = None, **extra: Any) -> dict[str, Any]: + tool_input: dict[str, Any] = {"id": _TICKET, "state": "Done", **extra} + if description is not None: + tool_input["description"] = description + return {"tool_name": "mcp__linear-server__save_issue", "tool_input": tool_input} + + +def _live(description: str) -> Any: + return lambda _t: { + "description": description, + "labels": [], + "attachment_urls": [_PR_URL], + } + + +def _decide( + call: dict[str, Any], clone: Path, monkeypatch: pytest.MonkeyPatch, **kw: Any +) -> Any: + """Run decide() with its PRODUCTION probe against the fixture OCC clone.""" + monkeypatch.setenv("OMNI_HOME", str(clone.parent)) + kw.setdefault("pr_fetcher", _merged) + kw.setdefault("receipt_lister", lambda _t: []) + return guard.decide(call, now=_NOW, **kw) + + +def _never(*_a: Any, **_k: Any) -> Any: + raise AssertionError("must not be called on this path") + + +# --------------------------------------------------------------------------- # +# AC1: NO_ACCEPTANCE_CHECKS is refused, merged PR or not +# --------------------------------------------------------------------------- # + + +def test_no_acceptance_checks_with_merged_pr_and_ticked_boxes_is_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The OMN-20180 incident, replayed: merged PR, boxes ticked, PASS receipts + for items that bind no criterion. Before OMN-20368 this was + ``durable_evidence:all_prs_merged``.""" + clone = _occ_clone( + tmp_path, + _UNBOUND_CONTRACT, + { + "dod-pr-3144": _receipt("dod-pr-3144", _UNBOUND_CONTRACT), + "dod-pr-3144-ci": _receipt("dod-pr-3144-ci", _UNBOUND_CONTRACT), + }, + ) + d = _decide( + _done(), + clone, + monkeypatch, + linear_fetcher=_live(_TICKED_DESCRIPTION), + ) + assert not d.allowed + assert "no_bound_dod_receipt" in d.reason + assert "AC1: no dod_evidence item" in d.reason + assert "AC2: no dod_evidence item" in d.reason + + +def test_no_acceptance_checks_no_contract_at_all_is_refused( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + (tmp_path / "onex_change_control").mkdir() + monkeypatch.setenv("OMNI_HOME", str(tmp_path)) + d = guard.decide( + _done(_TICKED_DESCRIPTION), + pr_fetcher=_merged, + receipt_lister=lambda _t: [], + now=_NOW, + ) + assert not d.allowed + assert "no_bound_dod_receipt" in d.reason + assert "no OCC contract" in d.reason + + +@pytest.mark.parametrize("label", ["close-if-done", "skip-merge-check"]) +def test_no_acceptance_checks_exemption_label_is_not_a_substitute(label: str) -> None: + d = guard.decide( + _done("decision only, nothing shipped", labels=[label]), + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(False, "no OCC contract"), + pr_fetcher=_never, + ) + assert not d.allowed + assert "no_bound_dod_receipt" in d.reason + + +def test_no_acceptance_checks_deploy_readback_marker_is_not_a_substitute() -> None: + desc = "deploy-readback-proven: rebuilt dev effects to dev-tip; probe exit 0\n" + d = guard.decide( + _done(desc), + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(False, "no OCC contract"), + pr_fetcher=_never, + ) + assert not d.allowed + assert "no_bound_dod_receipt" in d.reason + + +def test_no_acceptance_checks_a_failed_or_skipped_receipt_does_not_discharge( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + clone = _occ_clone( + tmp_path, + _BOUND_CONTRACT, + { + "dod-ac1": _receipt("dod-ac1", _BOUND_CONTRACT, status="FAIL"), + "dod-ac2": _receipt("dod-ac2", _BOUND_CONTRACT, status="SKIPPED"), + }, + ) + d = _decide(_done(), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert not d.allowed + assert "not a PASS" in d.reason + + +# --------------------------------------------------------------------------- # +# AC2: ticked boxes alone never pass; ticking without the receipt is refused +# --------------------------------------------------------------------------- # + + +def test_ticked_boxes_and_merged_pr_without_receipt_is_refused() -> None: + d = guard.decide( + _done(_TICKED_DESCRIPTION), + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(False, "nothing bound"), + pr_fetcher=_merged, + linear_fetcher=_live(_TICKED_DESCRIPTION), + receipt_lister=lambda _t: [], + ) + assert not d.allowed + assert "no_bound_dod_receipt" in d.reason + assert "Ticked boxes" in d.reason + + +def test_ticked_box_edit_without_receipt_is_refused() -> None: + call = { + "tool_name": "mcp__linear-server__save_issue", + "tool_input": {"id": _TICKET, "description": _TICKED_DESCRIPTION}, + } + d = guard.decide( + call, + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(False, "nothing bound"), + linear_fetcher=_live(_UNTICKED_DESCRIPTION), + ) + assert not d.allowed + assert d.reason.startswith("ac_tick_without_receipt") + assert "ticks 2 acceptance box(es)" in d.reason + + +def test_ticked_box_edit_by_patch_without_receipt_is_refused() -> None: + call = { + "tool_name": "mcp__linear-server__save_issue", + "tool_input": { + "id": _TICKET, + "state": "In Review", + "patch": [ + { + "op": "replace", + "old_string": "- [ ] AC1:", + "new_string": "- [x] AC1:", + } + ], + }, + } + d = guard.decide( + call, + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(False, "nothing bound"), + linear_fetcher=_live(_UNTICKED_DESCRIPTION), + ) + assert not d.allowed + assert d.reason.startswith("ac_tick_without_receipt") + assert "ticks 1 acceptance box(es)" in d.reason + + +def test_ticked_box_edit_when_live_description_unreadable_is_refused() -> None: + """Cannot compare before and after: every checked box counts as a tick.""" + call = { + "tool_name": "mcp__linear-server__update_issue", + "tool_input": {"id": _TICKET, "description": _TICKED_DESCRIPTION}, + } + d = guard.decide( + call, + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(False, "nothing bound"), + linear_fetcher=lambda _t: None, + ) + assert not d.allowed + assert d.reason.startswith("ac_tick_without_receipt") + + +def test_ticked_box_on_create_is_refused() -> None: + call = { + "tool_name": "mcp__linear-server__save_issue", + "tool_input": { + "team": "Omninode", + "title": "t", + "description": _TICKED_DESCRIPTION, + }, + } + d = guard.decide(call, occ_probe=_never, linear_fetcher=_never) + assert not d.allowed + assert d.reason.startswith("ac_tick_without_receipt") + + +def test_ticked_box_edit_with_bound_receipt_is_allowed() -> None: + call = { + "tool_name": "mcp__linear-server__save_issue", + "tool_input": {"id": _TICKET, "description": _TICKED_DESCRIPTION}, + } + d = guard.decide( + call, + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(True, "bound: AC1, AC2"), + linear_fetcher=_live(_UNTICKED_DESCRIPTION), + ) + assert d.allowed, d.reason + assert d.reason == "not_done_state" + + +def test_ticked_box_unchanged_edit_is_not_gated() -> None: + """Editing prose on a ticket whose boxes were already ticked ticks nothing.""" + call = { + "tool_name": "mcp__linear-server__save_issue", + "tool_input": { + "id": _TICKET, + "description": _TICKED_DESCRIPTION.replace("skips", "used to skip"), + }, + } + d = guard.decide(call, occ_probe=_never, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert d.allowed, d.reason + + +# --------------------------------------------------------------------------- # +# AC3: a receipt taken against a contract that has changed since is stale +# --------------------------------------------------------------------------- # + + +def test_stale_receipt_contract_entry_changed_is_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + edited = _BOUND_CONTRACT.replace("-k protected", "-k protected_surface") + clone = _occ_clone( + tmp_path, + edited, + { + # Both receipts were taken against the contract BEFORE the edit. + "dod-ac1": _receipt("dod-ac1", _BOUND_CONTRACT), + "dod-ac2": _receipt("dod-ac2", _BOUND_CONTRACT), + }, + ) + d = _decide(_done(), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert not d.allowed + assert "AC1: dod-ac1 receipt is stale" in d.reason + assert "AC2" not in d.reason.split("What is missing:")[1] + + +def test_stale_receipt_whole_file_hash_mismatch_is_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + clone = _occ_clone( + tmp_path, + _BOUND_CONTRACT, + { + item: _receipt( + item, _BOUND_CONTRACT, entry_hash=None, whole_hash="sha256:" + "0" * 64 + ) + for item in ("dod-ac1", "dod-ac2") + }, + ) + d = _decide(_done(), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert not d.allowed + assert "is stale: the contract changed" in d.reason + + +def test_stale_receipt_naming_no_contract_version_is_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + clone = _occ_clone( + tmp_path, + _BOUND_CONTRACT, + { + item: _receipt(item, _BOUND_CONTRACT, entry_hash=None) + for item in ("dod-ac1", "dod-ac2") + }, + ) + d = _decide(_done(), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert not d.allowed + assert "names no contract version" in d.reason + + +# --------------------------------------------------------------------------- # +# AC4: every criterion bound to a current PASS receipt is allowed +# --------------------------------------------------------------------------- # + + +def test_bound_pass_with_merged_pr_is_allowed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The receipts are older than the no-PR freshness window: a receipt about + merged code is judged by its commit and contract hash, not a state window.""" + clone = _occ_clone( + tmp_path, + _BOUND_CONTRACT, + { + item: _receipt(item, _BOUND_CONTRACT, run_timestamp="2026-09-01T00:00:00Z") + for item in ("dod-ac1", "dod-ac2") + }, + ) + d = _decide(_done(), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert d.allowed, d.reason + assert d.reason == "durable_evidence:occ_bound_receipts:all_prs_merged" + + +def test_bound_pass_whole_file_hash_match_is_allowed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + import hashlib + + whole = "sha256:" + hashlib.sha256(_BOUND_CONTRACT.encode()).hexdigest() + clone = _occ_clone( + tmp_path, + _BOUND_CONTRACT, + { + item: _receipt(item, _BOUND_CONTRACT, entry_hash=None, whole_hash=whole) + for item in ("dod-ac1", "dod-ac2") + }, + ) + d = _decide(_done(), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION)) + assert d.allowed, d.reason + + +def test_bound_pass_status_only_flip_with_unreadable_description_is_refused() -> None: + """No description, no live read: no criteria can be read, so nothing is bound.""" + d = guard.decide( + _done(), + occ_probe=lambda tid, desc: nbe.evaluate_bound_evidence( + tid, desc, nbe.OccTicketEvidence({"dod_evidence": []}), _NOW + ), + pr_fetcher=_never, + linear_fetcher=lambda _t: None, + ) + assert not d.allowed + assert "no acceptance criterion could be read" in d.reason + + +def test_bound_pass_done_call_cannot_drop_a_live_criterion( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The Done call rewrites the body to the bound criterion only. Today's AC2, + which nothing binds, still has to be met.""" + one_bound = _BOUND_CONTRACT.split(' - id: "dod-ac2"', maxsplit=1)[0] + clone = _occ_clone(tmp_path, one_bound, {"dod-ac1": _receipt("dod-ac1", one_bound)}) + rewritten = _TICKED_DESCRIPTION.split("- [X] AC2", maxsplit=1)[0] + d = _decide( + _done(rewritten), clone, monkeypatch, linear_fetcher=_live(_TICKED_DESCRIPTION) + ) + assert not d.allowed + assert "AC2: no dod_evidence item" in d.reason + + +def test_bound_pass_open_cited_pr_still_blocks() -> None: + d = guard.decide( + _done(_TICKED_DESCRIPTION), + occ_probe=lambda _t, _d: nbe.BoundEvidenceVerdict(True, "bound"), + pr_fetcher=lambda ref: ldv.PRStatus(ref=ref, state="OPEN", merge_state="CLEAN"), + receipt_lister=lambda _t: [], + ) + assert not d.allowed + assert "pr_not_merged" in d.reason + + +def test_bound_pass_state_by_id_is_refused() -> None: + """A Done passed by state id would skip every check if read as not-Done.""" + call = _done(_TICKED_DESCRIPTION) + call["tool_input"]["state"] = "2be3a0d0-646a-4349-946e-ca395cbb0109" + d = guard.decide(call, occ_probe=_never, pr_fetcher=_never, linear_fetcher=_never) + assert not d.allowed + assert d.reason.startswith("state_by_id") + + +# --------------------------------------------------------------------------- # +# AC5: fail closed -- own error, any cwd, lite mode, any hooks mask +# --------------------------------------------------------------------------- # + + +def test_fail_closed_main_refuses_when_decide_raises( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + def _boom(_call: dict[str, Any]) -> Any: + raise RuntimeError("synthetic guard defect") + + monkeypatch.setattr(guard, "decide", _boom) + monkeypatch.setattr(sys, "stdin", __import__("io").StringIO(json.dumps(_done()))) + assert guard.main() == 2 + err = capsys.readouterr().err + assert "guard_error" in err + assert "synthetic guard defect" in err + + +def _run_hook( + payload: dict[str, Any], cwd: Path, plugin_root: Path, env_extra: dict[str, str] +) -> subprocess.CompletedProcess[str]: + env = { + k: v + for k, v in os.environ.items() + if k not in ("CLAUDE_PROJECT_DIR", "PYTHONPATH") + } + env.update({"CLAUDE_PLUGIN_ROOT": str(plugin_root), "LINEAR_API_KEY": ""}) + env.update(env_extra) + return subprocess.run( + ["bash", str(plugin_root / "hooks" / "scripts" / _SCRIPT.name)], + input=json.dumps(payload), + capture_output=True, + text=True, + cwd=cwd, + env=env, + timeout=120, + check=False, + ) + + +@pytest.mark.parametrize( + "env_extra", + [ + {}, + {"OMNICLAUDE_MODE": "lite"}, + {"ONEX_HOOKS_MASK": "0"}, + ], + ids=["outside-repo", "lite-mode", "mask-all-off"], +) +def test_fail_closed_hook_refuses_in_any_cwd_mode_or_mask( + tmp_path: Path, env_extra: dict[str, str] +) -> None: + """The registered script, run from a directory that is no omninode repo, + refuses a receipt-less Done whatever the mode or mask says.""" + workspace = tmp_path / "workspace" + (workspace / "onex_change_control").mkdir(parents=True) + outside = tmp_path / "elsewhere" + outside.mkdir() + # A ticket id no workspace has: common.sh may load a real Linear key from + # the operator env file, and the live read must find nothing to rely on. + payload = _done("## Acceptance criteria\n\n- [x] AC1: done -- falsifier: x\n") + payload["tool_input"]["id"] = "OMN-99999999" + result = _run_hook( + payload, + outside, + _PLUGIN_ROOT, + {"OMNI_HOME": str(workspace), **env_extra}, + ) + assert result.returncode == 2, (result.stdout, result.stderr) + assert "no_bound_dod_receipt" in result.stderr + + +def test_fail_closed_hook_refuses_when_its_library_is_missing(tmp_path: Path) -> None: + root = tmp_path / "plugin" + (root / "hooks" / "scripts").mkdir(parents=True) + (root / "hooks" / "lib").mkdir(parents=True) + for name in ("pre_tool_use_done_flip_guard.sh", "common.sh"): + src = _PLUGIN_ROOT / "hooks" / "scripts" / name + (root / "hooks" / "scripts" / name).write_text(src.read_text()) + result = _run_hook(_done(), tmp_path, root, {}) + assert result.returncode == 2, (result.stdout, result.stderr) + assert "guard_error" in result.stderr + + +def test_fail_closed_occ_fetch_is_bounded_even_when_a_helper_holds_its_pipes( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A fetch whose helper outlives git must not outlast its deadline. + + Live, 2026-10-02: two Done-class writes passed while the hook ran past the + harness timeout on a loaded host. A ``git`` that leaves a sleeping child + behind is the shape a captured-output fetch waits on forever. + """ + import time + + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_git = fake_bin / "git" + fake_git.write_text("#!/bin/sh\nsleep 30 &\nsleep 30\n") + fake_git.chmod(0o755) + monkeypatch.setenv("PATH", f"{fake_bin}{os.pathsep}{os.environ['PATH']}") + start = time.monotonic() + nbe.bounded_fetch(tmp_path, timeout=1) + assert time.monotonic() - start < 10 diff --git a/tests/hooks/test_hook_inventory.py b/tests/hooks/test_hook_inventory.py index c2ce44d9cb..b8c55ff101 100644 --- a/tests/hooks/test_hook_inventory.py +++ b/tests/hooks/test_hook_inventory.py @@ -699,8 +699,10 @@ def test_lite_mode_darkness_is_declared_and_reported() -> None: if hook.lite_mode_exit and hook.enforcement } + # OMN-20368: the Done-flip gate left this set. It runs in lite mode, since + # a Linear Done is global state and the cwd says nothing about it. + assert "pre_tool_use_done_flip_guard.sh" not in enforcing_and_lite assert enforcing_and_lite == { - "pre_tool_use_done_flip_guard.sh", "pre_tool_use_lane_liveness_guard.sh", "post_tool_use_secret_redact_guard.sh", # OMN-13856 item 4: the actor-line comment guard exits in lite mode @@ -739,7 +741,9 @@ def test_the_lite_mode_declaration_is_checked_against_the_script(mirror: Path) - path = mirror / _INVENTORY_REL raw = yaml.safe_load(path.read_text()) for entry in raw["expected_hooks"]: - if entry["script"] == "pre_tool_use_done_flip_guard.sh": + # OMN-20368: the Done-flip gate no longer exits in lite mode, so the + # lane-liveness guard (which still does) is the subject of the flip. + if entry["script"] == "pre_tool_use_lane_liveness_guard.sh": entry["lite_mode_exit"] = False path.write_text(yaml.safe_dump(raw))