diff --git a/contracts/OMN-16824.yaml b/contracts/OMN-16824.yaml index 95685327867..ee6e6778f19 100644 --- a/contracts/OMN-16824.yaml +++ b/contracts/OMN-16824.yaml @@ -12,6 +12,9 @@ evidence_requirements: - kind: "ci" description: "PR #7393 CI checks green" command: "gh pr checks ${PR_NUMBER} --repo ${REPO}" + - kind: "tests" + description: "Shared cross-runner semantics case table executed against the hosted runner" + command: "uv run pytest tests/test_omn16824_test_passes_semantics.py -q" emergency_bypass: enabled: false justification: "" @@ -39,3 +42,40 @@ dod_evidence: checks: - check_type: "command" check_value: "gh pr view 7395 --repo OmniNode-ai/onex_change_control --json number,state" + - id: "dod-16824-one-semantic-executed" + description: >- + AC1/AC3/AC4. Runs the OMN-16824 suite in this repo. It executes the shared cross-runner case table + (tests/fixtures/check_type_runner_semantics.yaml, checked into omnimarket byte-for-byte and run + there against node_dod_verify) through the hosted runner, and pins the AC4 regression by execution + through run_compliance_check itself: with every gh call mocked to report a fully green PR, a contract + carrying a test_passes entry whose check_value fails returns exit 1, and the discriminating control + with a passing check_value returns exit 0. It also asserts the runner makes no gh pr checks call + at all, so the PR-status reading cannot come back by another route. + source: "manual" + status: "verified" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/test_omn16824_test_passes_semantics.py -q" + - id: "dod-16824-runner-regression-suite" + description: >- + AC2. Runs the compliance runner's own suite, which now pins the executed reading of test_passes + (replacing four tests that pinned the opposite), the bash -o pipefail invocation, and that command + and test_passes are one runner. Together with the suite above this is the cwd proof: a declared + cwd is honoured, an unresolvable one is NOT_EVALUATED, and the command demonstrably does not run + in the workspace instead (the case asserts no file the command would have created exists anywhere). + source: "manual" + status: "verified" + checks: + - check_type: "test_passes" + check_value: "uv run pytest tests/test_contract_compliance_check.py -q" + - id: "occ-self-bind-pr-7393" + description: >- + OCC self-binding for PR #7393, the in-repo PR carrying the fix, the shared case table, and the two + behaviour-proving legs above. Binds OMN-16824 to that OCC PR by pr_number so eligibility can resolve + a PASS receipt for the PR itself (OMN-14650). + source: "manual" + status: "verified" + checks: + - check_type: "command" + check_value: >- + gh pr view 7393 --repo OmniNode-ai/onex_change_control --json number,state,headRefName diff --git a/docs/CHECK_TYPES.md b/docs/CHECK_TYPES.md new file mode 100644 index 00000000000..82a73b47b9f --- /dev/null +++ b/docs/CHECK_TYPES.md @@ -0,0 +1,127 @@ +# `check_type` reference — what each DoD check actually does + +**Audience:** anyone authoring `dod_evidence` in `contracts/OMN-XXXX.yaml`. + +This page exists so you never have to read a runner's source to know what your +check will do. Two different gates execute the checks you write: + +| Runner | Where it runs | What it is | +|---|---|---| +| **Hosted** — `Contract Compliance Check` | every PR, in CI | `onex_change_control.scripts.contract_compliance_check` | +| **Local** — `node_dod_verify` | the local Done gate / autoclose evidence sweep | omnimarket `EvidenceCollector` | + +Both read the same contract bytes. **For check types both runners execute, this +page states one shared behaviour** — that is the OMN-16824 rule, and it is +enforced by an executed case table +(`tests/fixtures/check_type_runner_semantics.yaml`) that is checked into both +repos byte-for-byte and run against both runners. Rows marked `—` are hosted-only +surfaces until the local runner implements them. + +--- + +## The table + +| `check_type` | `check_value` is | Hosted runner | Local runner | +|---|---|---|---| +| `command` | a shell command | **executes it**; exit 0 = PASS | **executes it**; exit 0 = VERIFIED | +| `test_passes` | a shell command that runs tests | **executes it**; exit 0 = PASS | **executes it**; exit 0 = VERIFIED | +| `test_exists` | a glob | globs the workspace | — | +| `file_exists` | a glob | globs the workspace | checks the path exists | +| `grep` | `{pattern, path}` | greps the workspace | — | +| `endpoint` | a URL or path | curls / stats it | — | +| `behavior_proven` | prose attestation | non-executable WARN | non-executable | +| `semantic_grading` | a receipt path | receipt-gate surface | receipt-gate surface | + +### `test_passes` is an executed alias of `command` + +Write `test_passes` when the command you are running **is a test run**. It runs +exactly like `command`; the distinct name records author intent and is what the +proof-class classifier (OMN-15911) and the substance-floor tier deriver key on. + +It does **not** mean "this PR's CI is green". Until OMN-16824 the hosted runner +read it that way — it ignored `check_value` entirely and reported the PR's own +check-run states — so an entry claiming *"the new regression test passes"* +returned PASS whenever unrelated CI was green, including when the named test did +not exist. That reading is gone. If you genuinely want to assert PR CI state, +say so in the open: + +```yaml +- check_type: command + check_value: "gh pr checks 1719 --repo OmniNode-ai/omnimarket" +``` + +That is a **merge-state** proof, not a behaviour proof, and the classifier +scores it as such. + +--- + +## Where your command runs + +* **No `cwd` declared** → the command runs in the **product checkout under + test** (the repo whose PR is being gated), which the hosted runner passes as + `--workspace`. Relative paths resolve against it. +* **`cwd` declared** → the command runs **there**. Supported template tokens: + `${OMNI_HOME}`, `${PR_NUMBER}`, `${REPO}`, `${TICKET_ID}`. A relative `cwd` + resolves against the workspace; `..` segments are refused. +* **`cwd` declared but unresolvable** → the check is **`NOT_EVALUATED`**, and + the gate says so out loud. It is **never** rerouted to the workspace: running + your command in a different tree answers a different question under your + entry's name. + +The hosted gate checks out **one** repo. A `cwd` naming a sibling checkout +(`${OMNI_HOME}/omnimarket`) does not exist there and will be declined. That is +not a bug you should work around with a fabricated path — declare the item's +audience honestly: + +```yaml +- id: dod-cross-repo-behaviour + description: "…" + execution_scope: local_done_gate # OMN-15392 — hosted compliance will not evaluate this + checks: + - check_type: test_passes + check_value: "uv run pytest tests/unit/test_thing.py -q" + cwd: "${OMNI_HOME}/omnimarket" +``` + +`execution_scope: local_done_gate` items are reported `NOT-EVALUATED` by the +hosted gate by design, and do **not** count toward the hosted +`behavior_proving_count`. + +--- + +## The shell + +Commands execute under **`bash -o pipefail -c`** in both runners. A failing +first stage of a pipeline fails the check: + +```yaml +# FAILS when the gh call fails, which is what you want. +check_value: "gh api repos/OWNER/REPO/contents/x.py?ref= --jq .content | base64 -d | grep -q SYMBOL" +``` + +Under a plain `sh -c` this would have passed on `grep`'s exit code alone. + +--- + +## Two properties every check needs + +1. **It must execute.** Prose, an `echo`, or a bare `true` proves nothing. +2. **It must be able to go RED.** A check that reads only files this same + change authors (the receipt it is stamped in, the contract itself) confirms + what the author typed, not what the code does. Such checks are reported + `INERT` and demoted — see `onex_change_control.validation.evidence_admissibility` + and `admissible_evidence_guidance()`, which the runner prints on refusal. + +Timeout: 60s per check in the hosted runner (30s in `node_dod_verify`). A check +that needs longer belongs behind `execution_scope: local_done_gate`, not behind +a bigger number. + +--- + +## References + +* OMN-16824 — one semantic for `test_passes`; `cwd` honoured or declined +* OMN-15392 — `execution_scope: local_done_gate` +* OMN-15309 / OMN-14436 — the admissibility predicate and the grandfather ratchet +* OMN-15911 — the proof-class classifier that reads these checks +* `docs/TEMPLATE_GUIDE.md` — the full contract field reference diff --git a/docs/README.md b/docs/README.md index 0b788fe65ce..92449ecc514 100644 --- a/docs/README.md +++ b/docs/README.md @@ -16,6 +16,7 @@ New to this repo? Read in this order: 2. [CLAUDE.md](../CLAUDE.md) — developer context, naming conventions, development commands. 3. [Design: Drift Control System](design/DESIGN_DRIFT_CONTROL_SYSTEM.md) — the enforcement model. 4. [Template Guide](TEMPLATE_GUIDE.md) — how to author YAML artifacts. +5. [Check Types](CHECK_TYPES.md) — what each `dod_evidence` check actually executes. --- @@ -87,6 +88,7 @@ When automation creates Linear tickets from drift findings: |-----|---------| | [VERSIONING_POLICY.md](VERSIONING_POLICY.md) | Schema SemVer, immutability rules, breaking-change definition | | [TEMPLATE_GUIDE.md](TEMPLATE_GUIDE.md) | Field-by-field reference for `day_close.template.yaml` and `ticket_contract.template.yaml` | +| [CHECK_TYPES.md](CHECK_TYPES.md) | What every `dod_evidence` `check_type` does in each runner, the `cwd` rules, and the shell it executes under | | [EVAL_FRAMEWORK.md](EVAL_FRAMEWORK.md) | A/B evaluation framework: eval suites, comparator, metrics, verdicts | | [RECEIPT_LOCATIONS.md](RECEIPT_LOCATIONS.md) | Canonical DoD receipt location (`drift/dod_receipts/`) and migration from legacy path | | [wire-schema-contract-spec.md](wire-schema-contract-spec.md) | Wire schema contract specification | diff --git a/docs/TEMPLATE_GUIDE.md b/docs/TEMPLATE_GUIDE.md index c771238791d..9b4dea871ac 100644 --- a/docs/TEMPLATE_GUIDE.md +++ b/docs/TEMPLATE_GUIDE.md @@ -175,7 +175,8 @@ The Ticket Contract template (`ticket_contract.template.yaml`) is used to create #### `dod_evidence` - **Type**: List of objects (optional; omit if not needed) -- **Description**: Executable DoD checks. Each item's `checks[].check_value` is a shell command (`check_type: "command"`) run from the `onex_change_control` repo root by `scripts/ci/run_contract_compliance_check.py`; exit 0 = pass. +- **Description**: Executable DoD checks. For `check_type: "command"` and its executed alias `check_type: "test_passes"`, `checks[].check_value` is a shell command run by the hosted `Contract Compliance Check` **in the product checkout under test** — not the `onex_change_control` repo root — under `bash -o pipefail -c`; exit 0 = pass. A check may declare a `cwd`, which is honoured, or declined outright when it cannot be resolved. Other check types use the value shapes listed in `CHECK_TYPES.md`. +- **See [`CHECK_TYPES.md`](CHECK_TYPES.md)** for what every `check_type` does in each of the two runners, the `cwd` rules, and why `test_passes` is not a "PR CI is green" assertion (OMN-16824). - **`check_value` must be falsifiable — it must be able to fail if the work is actually wrong.** A command that reads the receipt file the check itself is stamped in and greps that file for `status: PASS` is **not evidence**: the receipt says PASS because the author wrote PASS, and the check confirms the author wrote PASS. It passes identically whether the code is correct or broken. > **⚠️ DO NOT DO THIS** (measured OMN-14417 at 2,137/6,915 = 31.3% of the live corpus, and 98.4% of contracts created in the last 7 days — this exact shape, copy-pasted PR to PR): diff --git a/drift/dod_receipts/OMN-16824/dod-16824-one-semantic-executed/test_passes.yaml b/drift/dod_receipts/OMN-16824/dod-16824-one-semantic-executed/test_passes.yaml new file mode 100644 index 00000000000..fcb6aa9d509 --- /dev/null +++ b/drift/dod_receipts/OMN-16824/dod-16824-one-semantic-executed/test_passes.yaml @@ -0,0 +1,32 @@ +--- +schema_version: "1.0.0" +ticket_id: "OMN-16824" +evidence_item_id: "dod-16824-one-semantic-executed" +check_type: "test_passes" +check_value: "uv run pytest tests/test_omn16824_test_passes_semantics.py -q" +contract_sha256: "sha256:55a93b881574c32b4ebef17c45aa516779008293c805c628a6c249400f63fa39" +contract_entry_sha256: "sha256:24922885657d4925110b7f00c943eb6a8954fd2bc83c479b39771265bf65e634" +status: PASS +run_timestamp: "2026-08-28T09:31:35Z" +commit_sha: "4426f1915f62b4e4f6e03a87a40ad1fd88a3c902" +runner: "testpasses-split-lane" +verifier: "live-command-readback" +probe_command: >- + uv run pytest tests/test_omn16824_test_passes_semantics.py -q -p no:cacheprovider --no-header -p no:warnings +probe_stdout: | + .................... [100%] + 20 passed in 0.43s +actual_output: >- + PASS: 20/20 green. The suite executes the shared cross-runner case table (tests/fixtures/check_type_runner_semantics.yaml, + checked into omnimarket and run there against node_dod_verify) through the hosted runner, and pins the + AC4 regression by execution through run_compliance_check itself. Falsifiability proven by execution + rather than asserted: this same suite was RED against the pre-fix runner in this worktree before the + change landed -- 13 of 20 failed, and the gate-level case printed "[+] test_passes: All 1 CI checks + green" for a contract whose test_passes check_value was a pytest invocation naming a file that does + not exist, returning exit 0. After the fix the identical input prints "[X] test_passes: Command failed + (exit 4)" and returns exit 1, while the discriminating control with a passing check_value still returns + 0. The suite also asserts the runner makes no gh pr checks call at all, so the substituted PR-status + question cannot return by another route. +exit_code: 0 +pr_number: 7393 +branch: "jonah/omn-16824-testpasses-executes" diff --git a/drift/dod_receipts/OMN-16824/dod-16824-runner-regression-suite/test_passes.yaml b/drift/dod_receipts/OMN-16824/dod-16824-runner-regression-suite/test_passes.yaml new file mode 100644 index 00000000000..ee013af6819 --- /dev/null +++ b/drift/dod_receipts/OMN-16824/dod-16824-runner-regression-suite/test_passes.yaml @@ -0,0 +1,30 @@ +--- +schema_version: "1.0.0" +ticket_id: "OMN-16824" +evidence_item_id: "dod-16824-runner-regression-suite" +check_type: "test_passes" +check_value: "uv run pytest tests/test_contract_compliance_check.py -q" +contract_sha256: "sha256:55a93b881574c32b4ebef17c45aa516779008293c805c628a6c249400f63fa39" +contract_entry_sha256: "sha256:d77bac0f0fc1fc714c1f9e61a868a0e12a0e5b8347c35e8b5d0c61c56153a702" +status: PASS +run_timestamp: "2026-08-28T09:31:35Z" +commit_sha: "4426f1915f62b4e4f6e03a87a40ad1fd88a3c902" +runner: "testpasses-split-lane" +verifier: "live-command-readback" +probe_command: >- + uv run pytest tests/test_contract_compliance_check.py -q -p no:cacheprovider --no-header -p no:warnings +probe_stdout: | + ....................................................................... [100%] + 71 passed in 0.35s +actual_output: >- + PASS: 71/71 green. This is the compliance runner's own suite. Four tests that pinned the OPPOSITE semantic + (test_passes reports whether the PR's own CI is green, ignoring check_value) are replaced by two that + pin the executed reading: a failing check_value BLOCKs even when every gh call is mocked to report a + green PR, and no gh pr call is made at all. Eleven pre-existing tests that asserted the sh -c invocation + shape were updated to the bash -o pipefail -c form the runner now uses, matching node_dod_verify since + OMN-15382. Together with the OMN-16824 suite this carries the AC2 cwd proof, including the case that + asserts an unresolvable cwd does NOT execute the command in the workspace instead (the probe command + would have created a file; none exists anywhere afterwards). +exit_code: 0 +pr_number: 7393 +branch: "jonah/omn-16824-testpasses-executes" diff --git a/drift/dod_receipts/OMN-16824/occ-self-bind-pr-7393/command.yaml b/drift/dod_receipts/OMN-16824/occ-self-bind-pr-7393/command.yaml new file mode 100644 index 00000000000..c3f3863f6c6 --- /dev/null +++ b/drift/dod_receipts/OMN-16824/occ-self-bind-pr-7393/command.yaml @@ -0,0 +1,28 @@ +--- +schema_version: "1.0.0" +ticket_id: "OMN-16824" +evidence_item_id: "occ-self-bind-pr-7393" +check_type: "command" +check_value: "gh pr view 7393 --repo OmniNode-ai/onex_change_control --json number,state,headRefName" +contract_sha256: "sha256:55a93b881574c32b4ebef17c45aa516779008293c805c628a6c249400f63fa39" +contract_entry_sha256: "sha256:5fc664865977b739518d05bac365d5d8aca8b0e12cde7d49ffbed2b244773e88" +status: PASS +run_timestamp: "2026-08-28T09:31:35Z" +commit_sha: "4426f1915f62b4e4f6e03a87a40ad1fd88a3c902" +runner: "testpasses-split-lane" +verifier: "live-command-readback" +probe_command: >- + gh api repos/OmniNode-ai/onex_change_control/pulls/7393/files --paginate --jq '.[].sha' +probe_stdout: | + b0b5effa3205d8937dcd73bfd9b6d43419731e3b 2f2b0b16a2c060309cc55031b83543641869974b + 92449ecc514aaaead0534781e26f5cb2690692b3 4eb5c997e4bb7b9c09f855936c18b2f450945c11 + 9abf0e8ab796816f90728f37e86c95f5dfd65c94 244ed2407964709185a6b3710362432c2471aabf + b763de14d75843ea4976e771b9f4122f89ebd84d cbe7b8dc0a90ad1a9b15f0f4a04f7fa3f36c2705 + 948658ed19a6fc2e575ff58f03f0f5e1d935628b +actual_output: >- + PASS: OCC self-bind for OMN-16824 (OCC#7393), the in-repo PR carrying the runner fix, the shared cross-runner + case table, the replaced regression tests, and the authoring documentation. The probe read the PR's + nine changed-file blob SHAs back from the live GitHub API rather than restating them. +exit_code: 0 +pr_number: 7393 +branch: "jonah/omn-16824-testpasses-executes" diff --git a/src/onex_change_control/models/model_dod_check.py b/src/onex_change_control/models/model_dod_check.py index 75de7363b3b..9abf0e8ab79 100644 --- a/src/onex_change_control/models/model_dod_check.py +++ b/src/onex_change_control/models/model_dod_check.py @@ -33,7 +33,12 @@ class ModelDodCheck(BaseModel): Each check has a type that determines how check_value is interpreted: - test_exists: check_value is a glob pattern for test files - - test_passes: check_value is a pytest marker or path to run + - test_passes: check_value is a shell command that runs tests -- an + EXECUTED alias of ``command`` (OMN-16824). Both the hosted Contract + Compliance Check and node_dod_verify run it under + ``bash -o pipefail -c`` and read its exit status. It does NOT mean + "this PR's CI is green"; assert that explicitly with + ``check_type: command`` + ``gh pr checks`` if you want it. - file_exists: check_value is a glob pattern for expected files - grep: check_value is a dict with 'pattern' and 'path' keys - command: check_value is a shell command (exit 0 = pass) @@ -49,8 +54,16 @@ class ModelDodCheck(BaseModel): execute under. The runner expands ``${OMNI_HOME}``, ``${PR_NUMBER}``, ``${REPO}``, and ``${TICKET_ID}`` template tokens before invocation, and is responsible for path-traversal containment checks. When ``cwd`` is - omitted the runner inherits its caller's working directory (legacy - behavior). + omitted the runner uses the product checkout under test (hosted) or + inherits its caller's working directory (local). + + OMN-16824: a declared ``cwd`` a runner cannot resolve makes the check + NOT_EVALUATED -- it is never rerouted to the runner's own workspace, since + running the command in a different tree answers a different question under + this entry's name. The hosted gate checks out one repo, so a cross-repo + ``cwd`` belongs to an item declaring + ``execution_scope: local_done_gate`` (OMN-15392). Authoring reference: + ``docs/CHECK_TYPES.md``. OMN-10078: replaces the brittle ``cd ${OMNI_HOME}/ && `` shell prefix introduced as a temporary fix in OMN-10049 / PR #448. diff --git a/src/onex_change_control/scripts/contract_compliance_check.py b/src/onex_change_control/scripts/contract_compliance_check.py index f065ee0eced..56c421f8a83 100755 --- a/src/onex_change_control/scripts/contract_compliance_check.py +++ b/src/onex_change_control/scripts/contract_compliance_check.py @@ -76,10 +76,6 @@ _RESULT_WARN = "WARN" _RESULT_BLOCK = "BLOCK" _RESULT_NOT_EVALUATED = "NOT_EVALUATED" -_SELF_STATUS_CHECK_NAMES = frozenset({"CI Summary", "Contract Compliance Check"}) -_NON_TERMINAL_STATUS_CHECK_STATES = frozenset( - {"EXPECTED", "IN_PROGRESS", "PENDING", "QUEUED"} -) _EXECUTION_SCOPE_HOSTED_AND_LOCAL = "hosted_and_local" _EXECUTION_SCOPE_LOCAL_DONE_GATE = "local_done_gate" _EXECUTION_SCOPES = frozenset( @@ -333,13 +329,16 @@ def _non_hermetic_reason(check: Any) -> str | None: exist on a CI runner. Returns ``None`` for a hermetic command (local file assertions, receipt greps, pure compute, loopback probes) so those still run. - Only ``check_type: command`` is inspected: its check_value is executed as a - shell command, so it is the only surface where these binaries actually run. - grep / file_exists / test_exists check_values are file/glob assertions that - may legitimately *contain* an IP or the word "docker" (e.g. grepping a - committed config), so scanning them would be a false-positive machine. + Only executed shell checks (``check_type: command`` and ``test_passes``) + are inspected. grep / file_exists / test_exists check_values are file/glob + assertions that may legitimately *contain* an IP or the word "docker" + (e.g. grepping a committed config), so scanning them would be a + false-positive machine. """ - if not isinstance(check, dict) or check.get("check_type") != "command": + if not isinstance(check, dict) or check.get("check_type") not in { + "command", + "test_passes", + }: return None command = str(check.get("check_value", "")) if not command.strip(): @@ -581,52 +580,6 @@ def _check_test_exists(check_value: Any, workspace: Path) -> tuple[str, str]: return _RESULT_BLOCK, f"No files found matching glob '{pattern}'" -def _check_test_passes( - _check_value: Any, - _workspace: Path, - pr_number: int, - repo: str, -) -> tuple[str, str]: - """check_type=test_passes: check via gh pr checks (CI must be green).""" - rc, out, err = _run( - ["gh", "pr", "checks", str(pr_number), "--repo", repo, "--json", "name,state"], - timeout=60, - ) - if rc != 0: - # gh pr checks fails if CI hasn't started yet -- warn, don't block - return ( - _RESULT_WARN, - f"Could not fetch PR checks (CI may not have started): {err}", - ) - - try: - checks = json.loads(out) - except json.JSONDecodeError: - return _RESULT_WARN, "Could not parse PR checks JSON" - - relevant_checks = [ - c for c in checks if c.get("name") not in _SELF_STATUS_CHECK_NAMES - ] - failures = [ - c - for c in relevant_checks - if c.get("state") - not in ("SUCCESS", "SKIPPED", "NEUTRAL", *_NON_TERMINAL_STATUS_CHECK_STATES) - ] - if failures: - names = ", ".join(c.get("name", "?") for c in failures) - return _RESULT_BLOCK, f"Failing CI checks: {names}" - pending = [ - c - for c in relevant_checks - if c.get("state") in _NON_TERMINAL_STATUS_CHECK_STATES - ] - if pending: - names = ", ".join(c.get("name", "?") for c in pending) - return _RESULT_WARN, f"CI checks still pending: {names}" - return _RESULT_PASS, f"All {len(checks)} CI checks green" - - def _check_file_exists(check_value: Any, workspace: Path) -> tuple[str, str]: """check_type=file_exists: check_value is a glob pattern.""" pattern = str(check_value) @@ -753,16 +706,113 @@ def _build_command_env( return {**os.environ, **overlay} -def _check_command( +def _resolve_check_cwd( + cwd_template: Any, + workspace: Path, + pr_number: int, + repo: str, + ticket_id: str, +) -> tuple[Path | None, str | None]: + """Resolve a check's declared ``cwd`` into a directory to execute in. + + Returns ``(directory, None)`` when the check may run, or + ``(None, decline_reason)`` when it may not. ``cwd`` omitted resolves to + ``workspace`` -- the product checkout under test, the pre-OMN-16824 + behaviour for every check. + + OMN-16824. Before this, ``_check_command`` passed ``cwd=workspace`` + unconditionally and never read the check's ``cwd`` at all, so a + cross-repo check silently resolved its paths against the wrong tree and + reported a verdict about a directory it was never pointed at. That is + worse than not running: it is an answer to a different question wearing + the contract entry's name. + + A declared ``cwd`` this runner cannot resolve is therefore DECLINED, never + rerouted. The hosted gate checks out exactly one product repo; a ``cwd`` + naming a sibling checkout (``${OMNI_HOME}/``) does not exist + here, and the honest report is that this gate did not evaluate the item -- + which is what ``execution_scope: local_done_gate`` (OMN-15392) declares up + front. + + Token substitution and containment mirror ``node_dod_verify``'s + ``_resolve_cwd`` (omnimarket) so both runners read the same field the same + way: ``${OMNI_HOME}``, ``${PR_NUMBER}``, ``${REPO}``, ``${TICKET_ID}``, + ``..`` rejected up front, and the resolved path must exist and be a + directory. + """ + if cwd_template is None: + return workspace, None + if not isinstance(cwd_template, str) or not cwd_template.strip(): + return None, ( + "NOT-EVALUATED [cwd] -- 'cwd' must be a non-empty string, got " + f"{type(cwd_template).__name__}; refusing to guess a directory." + ) + + if ".." in Path(cwd_template).parts: + return None, ( + f"NOT-EVALUATED [cwd] -- cwd path traversal not allowed: {cwd_template!r}" + ) + + substitutions = { + "OMNI_HOME": os.environ.get("OMNI_HOME", ""), + "PR_NUMBER": str(pr_number or os.environ.get("PR_NUMBER", "")), + "REPO": repo or os.environ.get("REPO", ""), + "TICKET_ID": ticket_id, + } + for token, value in substitutions.items(): + if f"${{{token}}}" in cwd_template and not value: + return None, ( + f"NOT-EVALUATED [cwd] -- cwd {cwd_template!r} references " + f"${{{token}}}, but that value is empty in this environment. " + "The hosted gate cannot execute this check; declare " + "execution_scope: local_done_gate (OMN-15392) so the local Done " + "gate owns it, or point cwd inside the checkout this PR changes." + ) + rendered = cwd_template + for token, value in substitutions.items(): + rendered = rendered.replace(f"${{{token}}}", value) + + if "${" in rendered or not rendered.strip(): + return None, ( + f"NOT-EVALUATED [cwd] -- cwd {cwd_template!r} has unresolved template " + f"tokens in this environment (rendered {rendered!r}). The hosted gate " + "cannot execute this check; declare " + "execution_scope: local_done_gate (OMN-15392) so the local Done gate " + "owns it, or point cwd inside the checkout this PR changes." + ) + + candidate = Path(rendered) + if not candidate.is_absolute(): + candidate = workspace / candidate + candidate = candidate.resolve() + + if not candidate.is_dir(): + return None, ( + f"NOT-EVALUATED [cwd] -- cwd {cwd_template!r} resolves to {candidate}, " + "which does not exist in this checkout. The hosted gate checks out " + "one product repo and will NOT reroute the command to its own " + "workspace (that would answer a different question). Declare " + "execution_scope: local_done_gate (OMN-15392) so the local Done gate " + "owns this item, or point cwd inside the checkout this PR changes." + ) + return candidate, None + + +def _check_command( # noqa: PLR0913 -- one parameter per contract-check field _check_value: Any, workspace: Path, pr_number: int = 0, repo: str = "", ticket_id: str = "", contracts_dir: Path | None = None, + cwd: Any = None, ) -> tuple[str, str]: """check_type=command: check_value is a shell command; exit 0 = pass. + The check's declared ``cwd`` is honoured (OMN-16824); when it cannot be + resolved the check is declined (NOT_EVALUATED), never rerouted to + ``workspace``. See ``_resolve_check_cwd``. + Supports both ``{pr}``/``{repo}``/``{ticket_id}`` and ``${PR_NUMBER}``/``${REPO}``/``${TICKET_ID}`` placeholders so contract YAML files don't hard-code PR numbers, repo names, or ticket IDs. Pre-substitutes @@ -782,6 +832,10 @@ def _check_command( f"Invalid --repo '{repo}': must match org/repo (alphanumeric, -, _, .)", ) + run_cwd, decline = _resolve_check_cwd(cwd, workspace, pr_number, repo, ticket_id) + if decline is not None: + return _RESULT_NOT_EVALUATED, decline + cmd_str = _substitute_tokens(str(_check_value), pr_number, repo, ticket_id) demoted = _maybe_demote_precommit(cmd_str) @@ -790,7 +844,17 @@ def _check_command( cmd_env = _build_command_env(cmd_str, pr_number, repo, ticket_id, contracts_dir) - rc, out, err = _run(["sh", "-c", cmd_str], timeout=60, cwd=workspace, env=cmd_env) + # ``bash -o pipefail``, not ``sh -c``: under ``sh`` a pipeline reports only + # its LAST stage's exit code, so ``gh api ... | grep -q X`` passes when the + # ``gh`` call itself failed. node_dod_verify has executed check_values this + # way since OMN-15382; matching it here is part of the OMN-16824 single + # semantic -- the same contract entry must not mean two things. + rc, out, err = _run( + ["bash", "-o", "pipefail", "-c", cmd_str], + timeout=60, + cwd=run_cwd, + env=cmd_env, + ) if rc == 0: return _RESULT_PASS, f"Command succeeded: {cmd_str[:80]}" output_snippet = (out + err)[:200] @@ -800,6 +864,44 @@ def _check_command( ) +def _check_test_passes( # noqa: PLR0913 -- signature is _check_command's, exactly + check_value: Any, + workspace: Path, + pr_number: int = 0, + repo: str = "", + ticket_id: str = "", + contracts_dir: Path | None = None, + cwd: Any = None, +) -> tuple[str, str]: + """check_type=test_passes: an EXECUTED alias of ``check_type: command``. + + OMN-16824 -- this runner used to ignore ``check_value`` entirely and report + whether the PR's own CI was green. ``node_dod_verify`` (omnimarket) has + always executed ``check_value`` and honoured ``cwd``, so one contract entry + was a behaviour proof to one gate and a PR-status proxy to the other. Worse, + the PR-status reading could never go RED for the reason the entry claimed: + an entry asserting "the new test passes" returned PASS whenever unrelated CI + was green, including when the named test did not exist. + + One semantic now: ``test_passes`` runs ``check_value`` and reads its exit + status, exactly like ``command``. The alias survives because it states the + author's intent (this command is a test run) and is what + ``derive_proof_tier`` and the OMN-15911 proof classifier key on; it is not a + different question. A contract that genuinely wants to assert PR CI state + must say so with an explicit ``check_type: command`` probe of ``gh pr + checks`` -- visible in the contract instead of hidden behind this name. + """ + return _check_command( + check_value, + workspace, + pr_number, + repo, + ticket_id, + contracts_dir, + cwd, + ) + + def _check_endpoint(check_value: Any, workspace: Path) -> tuple[str, str]: """check_type=endpoint: check_value is a URL or local path.""" target = str(check_value) @@ -818,6 +920,12 @@ def _check_endpoint(check_value: Any, workspace: Path) -> tuple[str, str]: return _RESULT_BLOCK, f"Path not found: {target}" +# OMN-16824: check types whose check_value is shell text this runner EXECUTES, +# honouring the check's ``cwd``. They share one signature and one dispatch +# branch precisely so a future edit cannot give one of them a different meaning +# without giving it to the other. +_EXECUTED_COMMAND_CHECK_TYPES: frozenset[str] = frozenset({"command", "test_passes"}) + _CHECK_RUNNERS: dict[str, Any] = { "test_exists": _check_test_exists, "test_passes": _check_test_passes, @@ -877,7 +985,10 @@ def _run_single_check( runner = _CHECK_RUNNERS.get(check_type) if runner is None: return check_type, _RESULT_WARN, f"Unknown check_type '{check_type}'" - if check_type == "command": + if check_type in _EXECUTED_COMMAND_CHECK_TYPES: + # OMN-16824: ONE dispatch for both, because they are one semantic -- + # execute check_value, honour the check's cwd. Two branches here is + # exactly how the two readings drifted apart. result, detail = runner( check_value, workspace, @@ -885,9 +996,8 @@ def _run_single_check( context.repo, context.ticket_id, context.contracts_dir, + check.get("cwd"), ) - elif check_type == "test_passes": - result, detail = runner(check_value, workspace, context.pr_number, context.repo) else: result, detail = runner(check_value, workspace) return check_type, result, detail @@ -1164,6 +1274,25 @@ def _outside_diff_state(changed_paths: frozenset[str]) -> str: ) +def _nothing_proven_summary(warns: int, not_evaluated: int, total: int) -> str: + """The line printed when no check FAILED but none PROVED anything either. + + Do not call this "all checks satisfied" -- nothing was proven, and saying so + is the declaration-in-place-of-verification the ratchet exists to remove. + OMN-16824 folded NOT_EVALUATED into this branch: a contract whose every + check DECLINED (an unresolvable cross-repo ``cwd``) proved exactly as little + as one whose every check WARNed, and previously printed "All executable DoD + checks satisfied." + """ + parts = [f"{warns}/{total} were WARN"] + if not_evaluated: + parts.append(f"{not_evaluated}/{total} were NOT_EVALUATED") + return ( + f"[PASS] No enforceable DoD check failed, but {', '.join(parts)} " + "and 0 proved anything about the product." + ) + + def run_compliance_check( pr_number: int, repo: str, @@ -1277,15 +1406,8 @@ def run_compliance_check( ) return 1 - if warns and not passes: - # Do not call this "all checks satisfied" -- nothing was proven. Saying - # so is the same declaration-in-place-of-verification this ticket exists - # to remove. - print( - f"[PASS] No enforceable DoD check failed, but {warns}/{total} were " - "WARN and 0 proved anything about the product.", - flush=True, - ) + if (warns or not_evaluated) and not passes: + print(_nothing_proven_summary(warns, not_evaluated, total), flush=True) return 0 print("[PASS] All executable DoD checks satisfied.", flush=True) diff --git a/tests/fixtures/check_type_runner_semantics.yaml b/tests/fixtures/check_type_runner_semantics.yaml new file mode 100644 index 00000000000..b763de14d75 --- /dev/null +++ b/tests/fixtures/check_type_runner_semantics.yaml @@ -0,0 +1,108 @@ +--- +# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +# +# OMN-16824 -- the ONE cross-runner semantic for check_type: test_passes. +# +# This file is the shared, executable statement of what a `test_passes` (and +# `command`) DoD check means. It is checked in BYTE-IDENTICALLY to two repos and +# executed against a different runner in each: +# +# onex_change_control tests/fixtures/check_type_runner_semantics.yaml +# -> onex_change_control.scripts.contract_compliance_check +# (the hosted "Contract Compliance Check" gate) +# omnimarket tests/fixtures/check_type_runner_semantics.yaml +# -> omnimarket node_dod_verify EvidenceCollector +# (the local Done gate) +# +# Both harnesses assert the SAME `expected` verdict for every case, and both +# pin the sha256 of this file, so a one-sided edit fails that repo's test. +# +# THE SEMANTIC, stated once: +# * `test_passes` is an EXECUTED alias of `command`. Both runners run +# `check_value` in a shell and read its exit status. Neither runner +# substitutes a different question -- in particular, neither asks whether +# the PR's own CI is green. +# * a declared `cwd` is HONOURED. A `cwd` the runner cannot resolve REFUSES +# the check; it is never silently rerouted to the runner's own workspace. +# * the shell is `bash -o pipefail`, so a failing first stage of a pipeline +# fails the check. +# +# Verdict vocabulary (deliberately coarse, so the two runners' different +# reporting shapes can still be compared): +# verified -- the runner reports the check satisfied (OCC PASS / +# node_dod_verify VERIFIED) +# refused -- the runner does NOT report it satisfied, whether it failed the +# command or declined to run it (OCC BLOCK or NOT_EVALUATED / +# node_dod_verify FAILED) +schema_version: 1 + +# Directories and files the harness materialises under the fixture root before +# running any case. The fixture root is exported as ${OMNI_HOME} so `cwd` +# templates below resolve identically in both repos. +fixture_files: + - product/marker.txt + - product/sub/only_here.txt + +cases: + - id: executing-a-failing-command-refuses + reason: >- + The anchor case. A test_passes entry whose check_value fails must refuse even when the PR's own + CI is entirely green. Before OMN-16824 the hosted runner returned PASS here because it never looked + at check_value. + check_type: test_passes + check_value: "false" + cwd: "${OMNI_HOME}/product" + expected: refused + + - id: executing-a-passing-command-verifies + reason: The green control for the case above. + check_type: test_passes + check_value: "test -f marker.txt" + cwd: "${OMNI_HOME}/product" + expected: verified + + - id: command-check_type-agrees-with-test_passes + reason: >- + test_passes is an alias of command, so the same check_value must produce the same verdict under + either check_type. + check_type: command + check_value: "false" + cwd: "${OMNI_HOME}/product" + expected: refused + + - id: declared-cwd-is-honoured + reason: >- + The check_value names a file that exists ONLY under the declared cwd, so a runner that ignored cwd + could not pass this case. + check_type: test_passes + check_value: "test -f only_here.txt" + cwd: "${OMNI_HOME}/product/sub" + expected: verified + + - id: declared-cwd-is-not-a-search-path + reason: >- + The mirror of the case above -- the same file, a cwd one level up. A runner that fell back to some + other directory would wrongly verify. + check_type: test_passes + check_value: "test -f only_here.txt" + cwd: "${OMNI_HOME}/product" + expected: refused + + - id: unresolvable-cwd-refuses-instead-of-relocating + reason: >- + A cwd the runner cannot resolve (the cross-repo case) must refuse. Running the command somewhere + else answers a different question about a different tree, which is exactly the OMN-16824 defect. + check_type: test_passes + check_value: "test -f marker.txt" + cwd: "${OMNI_HOME}/no_such_checkout" + expected: refused + + - id: pipeline-first-stage-failure-refuses + reason: >- + Both runners execute under `bash -o pipefail`, so a failing first stage is not masked by a succeeding + last stage. + check_type: test_passes + check_value: "false | cat" + cwd: "${OMNI_HOME}/product" + expected: refused diff --git a/tests/test_contract_compliance_check.py b/tests/test_contract_compliance_check.py index f2576ce967c..9e4ca37ba80 100644 --- a/tests/test_contract_compliance_check.py +++ b/tests/test_contract_compliance_check.py @@ -149,94 +149,53 @@ def test_check_test_exists_block(tmp_path: Path) -> None: assert result == _RESULT_BLOCK -def test_check_test_passes_ignores_own_contract_compliance_context( +def test_check_test_passes_executes_check_value_and_ignores_pr_ci_state( tmp_path: Path, ) -> None: - checks_json = ( - '[{"name":"Contract Compliance Check","state":"FAILURE"},' - '{"name":"tests+coverage (shadow)","state":"SUCCESS"}]' - ) - with patch( - "onex_change_control.scripts.contract_compliance_check._run", - return_value=(0, checks_json, ""), - ): - result, detail = _check_test_passes( - None, - tmp_path, - pr_number=5976, - repo="OmniNode-ai/onex_change_control", - ) + """OMN-16824: ``test_passes`` is an EXECUTED alias of ``command``. - assert result == _RESULT_PASS - assert "green" in detail + This test replaces four tests that pinned the OPPOSITE semantic -- that + ``test_passes`` ignored ``check_value`` and reported whether the PR's own CI + was green. That reading made the entry unfalsifiable for the thing it + claimed: a contract asserting "the new test passes" returned PASS whenever + unrelated CI was green. Both runners now execute ``check_value``. + """ + calls: list[list[str]] = [] + def fake_run(cmd: list[str], **_kwargs: object) -> tuple[int, str, str]: + calls.append(cmd) + if cmd[:2] == ["gh", "pr"]: + # If the runner ever asks this question again, it gets a green + # answer -- and must still BLOCK on the failing check_value below. + return 0, '[{"name":"CI","state":"SUCCESS"}]', "" + return 1, "", "boom" -def test_check_test_passes_ignores_ci_summary_aggregate_context( - tmp_path: Path, -) -> None: - checks_json = ( - '[{"name":"Contract Compliance Check","state":"FAILURE"},' - '{"name":"CI Summary","state":"FAILURE"},' - '{"name":"tests+coverage (shadow)","state":"SUCCESS"}]' - ) with patch( "onex_change_control.scripts.contract_compliance_check._run", - return_value=(0, checks_json, ""), + side_effect=fake_run, ): result, detail = _check_test_passes( - None, + "uv run pytest tests/test_nope.py", tmp_path, pr_number=5976, repo="OmniNode-ai/onex_change_control", ) - assert result == _RESULT_PASS - assert "green" in detail - - -def test_check_test_passes_blocks_non_self_failed_context(tmp_path: Path) -> None: - checks_json = ( - '[{"name":"Contract Compliance Check","state":"FAILURE"},' - '{"name":"CI Summary","state":"FAILURE"},' - '{"name":"tests+coverage (shadow)","state":"FAILURE"}]' + assert result == _RESULT_BLOCK, detail + assert not any(cmd[:2] == ["gh", "pr"] for cmd in calls), ( + f"runner substituted the PR-CI question: {calls}" ) - with patch( - "onex_change_control.scripts.contract_compliance_check._run", - return_value=(0, checks_json, ""), - ): - result, detail = _check_test_passes( - None, - tmp_path, - pr_number=5976, - repo="OmniNode-ai/onex_change_control", - ) - - assert result == _RESULT_BLOCK - assert "tests+coverage (shadow)" in detail - assert "Contract Compliance Check" not in detail - assert "CI Summary" not in detail - + assert calls[-1][-1] == "uv run pytest tests/test_nope.py" -def test_check_test_passes_warns_on_pending_non_self_context(tmp_path: Path) -> None: - checks_json = ( - '[{"name":"Contract Compliance Check","state":"FAILURE"},' - '{"name":"CI Summary","state":"PENDING"},' - '{"name":"tests+coverage (shadow)","state":"IN_PROGRESS"}]' - ) - with patch( - "onex_change_control.scripts.contract_compliance_check._run", - return_value=(0, checks_json, ""), - ): - result, detail = _check_test_passes( - None, - tmp_path, - pr_number=5976, - repo="OmniNode-ai/onex_change_control", - ) - assert result == _RESULT_WARN - assert "tests+coverage (shadow)" in detail - assert "CI Summary" not in detail +def test_check_test_passes_and_command_are_the_same_runner(tmp_path: Path) -> None: + """The two check types must not be able to drift apart again.""" + (tmp_path / "marker.txt").write_text("x\n") + for check_type_runner in (_check_test_passes, _check_command): + result, detail = check_type_runner("test -f marker.txt", tmp_path) + assert result == _RESULT_PASS, detail + failing, _ = check_type_runner("test -f absent.txt", tmp_path) + assert failing == _RESULT_BLOCK def test_check_file_exists_pass(tmp_path: Path) -> None: @@ -303,11 +262,14 @@ def fake_run(cmd: list[str], **_kwargs: object) -> tuple[int, str, str]: assert result == _RESULT_PASS assert captured, "No subprocess call captured — _run was never invoked" - # The final sh -c call must contain the substituted shell string + # The final shell call must contain the substituted shell string. + # OMN-16824: the runner invokes ``bash -o pipefail -c `` (was + # ``sh -c ``) so a failing first pipeline stage is not masked, matching + # node_dod_verify. The command text is the LAST argv element either way. shell_cmd = captured[-1] - assert shell_cmd[0] == "sh", f"Unexpected command: {shell_cmd}" - assert shell_cmd[1] == "-c", f"Unexpected command: {shell_cmd}" - shell_str = shell_cmd[2] + assert shell_cmd[0] == "bash", f"Unexpected command: {shell_cmd}" + assert shell_cmd[-2] == "-c", f"Unexpected command: {shell_cmd}" + shell_str = shell_cmd[-1] assert "586" in shell_str, f"PR number not substituted in shell cmd: {shell_str!r}" assert "OmniNode-ai/omnidash" in shell_str, f"repo not substituted: {shell_str!r}" assert "{pr}" not in shell_str, f"Literal {{pr}} not replaced: {shell_str!r}" @@ -343,7 +305,7 @@ def fake_run(cmd: list[str], **_kwargs: object) -> tuple[int, str, str]: ) assert result == _RESULT_PASS - shell_str = captured[-1][2] + shell_str = captured[-1][-1] expected = ( "gh pr checks 452 --repo OmniNode-ai/onex_change_control && echo OMN-10086" ) @@ -512,7 +474,7 @@ def fake_run(cmd: list[str], **_kwargs: object) -> tuple[int, str, str]: assert result == _RESULT_PASS assert calls == [ ["which", "pre-commit"], - ["sh", "-c", "pre-commit run --all-files"], + ["bash", "-o", "pipefail", "-c", "pre-commit run --all-files"], ] @@ -1530,6 +1492,15 @@ def test_non_hermetic_rejects_ssh() -> None: assert "status: PASS" in reason +def test_non_hermetic_rejects_test_passes_alias() -> None: + """OMN-16824: test_passes executes, so it gets the same hermetic guard.""" + reason = _non_hermetic_reason( + {"check_type": "test_passes", "check_value": "docker ps"} + ) + assert reason is not None + assert "docker" in reason + + def test_non_hermetic_rejects_live_docker() -> None: """`docker exec` against a live daemon (absent in CI) is rejected.""" reason = _non_hermetic_reason( @@ -1762,7 +1733,7 @@ def fake_run(cmd: list[str], **_kwargs: object) -> tuple[int, str, str]: repo=_UNRELATED_RUNNER_REPO, ) assert captured, "No subprocess call captured -- _run was never invoked" - return captured[-1][2] + return captured[-1][-1] @pytest.mark.parametrize( diff --git a/tests/test_omn16824_test_passes_semantics.py b/tests/test_omn16824_test_passes_semantics.py new file mode 100644 index 00000000000..9240c1a0abe --- /dev/null +++ b/tests/test_omn16824_test_passes_semantics.py @@ -0,0 +1,379 @@ +# SPDX-FileCopyrightText: 2026 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""OMN-16824 -- one semantic for ``check_type: test_passes``. + +Before this ticket, ``test_passes`` meant two incompatible things: + +* ``node_dod_verify`` (omnimarket) EXECUTED ``check_value`` and honoured the + check's ``cwd``; +* the hosted ``Contract Compliance Check`` in this repo IGNORED ``check_value`` + entirely and asked instead whether the PR's own CI was green. + +The same contract entry was therefore a behaviour proof to one gate and a +PR-status proxy to the other, and a cross-repo behaviour check was structurally +unrunnable in the hosted gate because ``_check_command`` passed +``cwd=workspace`` unconditionally. + +AC1 option (a) is implemented: the hosted runner EXECUTES the check the same +way ``node_dod_verify`` does, honours the declared ``cwd``, and DECLINES loudly +(``NOT_EVALUATED``) when a declared ``cwd`` cannot be resolved in the hosted +checkout rather than silently rerouting the command to its own workspace. + +The case table in ``tests/fixtures/check_type_runner_semantics.yaml`` is shared +byte-for-byte with omnimarket and is executed against BOTH runners, one in each +repo -- that is the AC3 ambiguity test. +""" + +from __future__ import annotations + +import hashlib +import json +import textwrap +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import pytest +import yaml + +from onex_change_control.scripts.contract_compliance_check import ( + _RESULT_BLOCK, + _RESULT_NOT_EVALUATED, + _RESULT_PASS, + _CheckContext, + _run, + _run_single_check, + run_compliance_check, +) + +_CASE_TABLE = Path(__file__).parent / "fixtures" / "check_type_runner_semantics.yaml" + +# Pinned so an edit to the shared table is a deliberate, reviewed act in BOTH +# repos. omnimarket pins the identical digest over its identical copy; the two +# constants are the mechanical link between the two halves of this test. +# +# The digest is taken over the PARSED table in canonical JSON, not the file +# bytes: each repo's yamlfmt reflows YAML on commit, and a reflow is not a +# change of meaning. Content edits still break it in both repos. +CASE_TABLE_DIGEST = "338cc633d858d71e19e1fc6b2ac76a54c9596e490f1c6a2fd8211b9e845e79c4" + + +def _case_table_digest(table: dict[str, Any]) -> str: + return hashlib.sha256( + json.dumps(table, sort_keys=True, separators=(",", ":")).encode() + ).hexdigest() + + +def _load_cases() -> tuple[dict[str, Any], list[dict[str, Any]]]: + table = yaml.safe_load(_CASE_TABLE.read_text()) + return table, list(table["cases"]) + + +def _context() -> _CheckContext: + return _CheckContext( + pr_number=1, + repo="OmniNode-ai/onex_change_control", + ticket_id="OMN-16824", + contracts_dir=None, + is_legacy=False, + changed_paths=frozenset(), + ) + + +def _materialise(root: Path, table: dict[str, Any]) -> None: + for rel in table["fixture_files"]: + target = root / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("fixture\n") + + +# --------------------------------------------------------------------------- +# AC3 -- the shared cross-runner case table, executed against THIS runner +# --------------------------------------------------------------------------- + + +def test_case_table_content_is_pinned() -> None: + """The shared table cannot drift in one repo without failing that repo.""" + table, _ = _load_cases() + assert _case_table_digest(table) == CASE_TABLE_DIGEST, ( + "tests/fixtures/check_type_runner_semantics.yaml changed. It is shared " + "with omnimarket: update BOTH copies and BOTH pinned " + "digests, or the two runners can diverge again silently." + ) + + +@pytest.mark.parametrize("case", _load_cases()[1], ids=lambda c: str(c["id"])) +def test_hosted_runner_matches_the_shared_semantic( + case: dict[str, Any], + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + table, _ = _load_cases() + _materialise(tmp_path, table) + monkeypatch.setenv("OMNI_HOME", str(tmp_path)) + + check = { + "check_type": case["check_type"], + "check_value": case["check_value"], + } + if case.get("cwd"): + check["cwd"] = case["cwd"] + + _, result, detail = _run_single_check(check, tmp_path, _context()) + + observed = "verified" if result == _RESULT_PASS else "refused" + assert observed == case["expected"], ( + f"case {case['id']}: expected {case['expected']}, got {result} -- {detail}\n" + f"{case['reason']}" + ) + + +# --------------------------------------------------------------------------- +# AC1/AC4 -- the runner executes check_value and never substitutes PR CI state +# --------------------------------------------------------------------------- + + +def test_test_passes_blocks_on_a_failing_check_value_with_green_pr_ci( + tmp_path: Path, +) -> None: + """The regression AC4 pins. + + A ``test_passes`` entry whose ``check_value`` fails must BLOCK. Under the + pre-OMN-16824 behaviour this returned PASS whenever the PR's own CI was + green -- the runner never ran the command at all. + """ + calls: list[list[str]] = [] + + def _spy(cmd: list[str], *args: Any, **kwargs: Any) -> tuple[int, str, str]: + calls.append(list(cmd)) + return _run(cmd, *args, **kwargs) + + with patch( + "onex_change_control.scripts.contract_compliance_check._run", + side_effect=_spy, + ): + check_type, result, detail = _run_single_check( + {"check_type": "test_passes", "check_value": "false"}, + tmp_path, + _context(), + ) + + assert check_type == "test_passes" + assert result == _RESULT_BLOCK, detail + # ...and it did not answer a different question on the way. + flat = [" ".join(c) for c in calls] + assert not any("pr checks" in c or ("pr" in c and "checks" in c) for c in flat), ( + f"the runner consulted the PR's CI state instead of executing " + f"check_value: {flat}" + ) + + +def test_test_passes_passes_on_a_succeeding_check_value(tmp_path: Path) -> None: + (tmp_path / "marker.txt").write_text("x\n") + _, result, detail = _run_single_check( + {"check_type": "test_passes", "check_value": "test -f marker.txt"}, + tmp_path, + _context(), + ) + assert result == _RESULT_PASS, detail + + +# --------------------------------------------------------------------------- +# AC2 -- cwd is read, and an unresolvable cwd declines instead of relocating +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize("check_type", ["command", "test_passes"]) +def test_declared_cwd_is_honoured(check_type: str, tmp_path: Path) -> None: + sub = tmp_path / "product" / "sub" + sub.mkdir(parents=True) + (sub / "only_here.txt").write_text("x\n") + + _, result, detail = _run_single_check( + { + "check_type": check_type, + "check_value": "test -f only_here.txt", + "cwd": str(sub), + }, + tmp_path, + _context(), + ) + assert result == _RESULT_PASS, detail + + +@pytest.mark.parametrize("check_type", ["command", "test_passes"]) +def test_unresolvable_cwd_declines_without_executing_in_the_workspace( + check_type: str, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A cross-repo cwd absent from the hosted checkout must not be rerouted. + + The command would create ``ran.txt`` wherever it executed; the runner must + decline, so no such file exists anywhere afterwards. + """ + monkeypatch.setenv("OMNI_HOME", str(tmp_path)) + _, result, detail = _run_single_check( + { + "check_type": check_type, + "check_value": "touch ran.txt", + "cwd": "${OMNI_HOME}/omnimarket", + }, + tmp_path, + _context(), + ) + assert result == _RESULT_NOT_EVALUATED, detail + assert "local_done_gate" in detail + assert not (tmp_path / "ran.txt").exists() + assert not list(tmp_path.rglob("ran.txt")) + + +@pytest.mark.parametrize("check_type", ["command", "test_passes"]) +def test_cwd_path_traversal_is_refused(check_type: str, tmp_path: Path) -> None: + _, result, detail = _run_single_check( + { + "check_type": check_type, + "check_value": "true", + "cwd": "../../etc", + }, + tmp_path, + _context(), + ) + assert result == _RESULT_NOT_EVALUATED, detail + assert "traversal" in detail.lower() + + +@pytest.mark.parametrize("check_type", ["command", "test_passes"]) +def test_unresolved_cwd_template_token_is_refused( + check_type: str, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("OMNI_HOME", raising=False) + _, result, detail = _run_single_check( + { + "check_type": check_type, + "check_value": "true", + "cwd": "${OMNI_HOME}/omnimarket", + }, + tmp_path, + _context(), + ) + assert result == _RESULT_NOT_EVALUATED, detail + + +@pytest.mark.parametrize("check_type", ["command", "test_passes"]) +def test_empty_supported_cwd_template_value_is_refused( + check_type: str, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("OMNI_HOME", raising=False) + marker = tmp_path / "omnimarket" / "wrong-workspace-marker.txt" + marker.parent.mkdir() + marker.write_text("x\n") + + _, result, detail = _run_single_check( + { + "check_type": check_type, + "check_value": "test -f wrong-workspace-marker.txt", + "cwd": "${OMNI_HOME}/omnimarket", + }, + tmp_path, + _CheckContext( + pr_number=0, + repo="", + ticket_id="OMN-16824", + contracts_dir=None, + is_legacy=False, + changed_paths=frozenset(), + ), + ) + assert result == _RESULT_NOT_EVALUATED, detail + assert "OMNI_HOME" in detail + assert "empty" in detail + + +# --------------------------------------------------------------------------- +# AC4 -- falsifiability through the gate's own entry point, not a helper +# --------------------------------------------------------------------------- + + +def _write_contract(contracts_dir: Path, body: str) -> None: + contracts_dir.mkdir(parents=True, exist_ok=True) + (contracts_dir / "OMN-16824.yaml").write_text(textwrap.dedent(body)) + + +@pytest.fixture +def green_pr_ci() -> Any: + """Every ``gh`` call the gate makes reports a fully green PR.""" + pr_json = ( + '{"title": "feat(OMN-16824): x", "headRefName": "jonah/omn-16824", "body": ""}' + ) + + def _fake(cmd: list[str], *args: Any, **kwargs: Any) -> tuple[int, str, str]: + if cmd[:2] == ["gh", "pr"] and "view" in cmd: + return 0, pr_json, "" + if cmd[:2] == ["gh", "pr"] and "checks" in cmd: + return 0, '[{"name":"CI","state":"SUCCESS"}]', "" + if cmd[:2] == ["gh", "api"]: + return 0, "", "" + return _run(cmd, *args, **kwargs) + + with patch( + "onex_change_control.scripts.contract_compliance_check._run", + side_effect=_fake, + ): + yield + + +@pytest.mark.usefixtures("green_pr_ci") +def test_gate_goes_red_on_a_failing_test_passes_entry(tmp_path: Path) -> None: + """AC4 by execution: the whole gate, not just the check helper.""" + contracts = tmp_path / "contracts" + _write_contract( + contracts, + """ + ticket_id: OMN-16824 + dod_evidence: + - id: dod-deliberately-failing + description: A test_passes entry whose check_value fails on purpose + checks: + - check_type: test_passes + check_value: "pytest tests/test_omn16824_no_such_test.py" + """, + ) + exit_code = run_compliance_check( + pr_number=1, + repo="OmniNode-ai/onex_change_control", + contracts_dir=contracts, + workspace=tmp_path, + ) + assert exit_code == 1 + + +@pytest.mark.usefixtures("green_pr_ci") +def test_gate_is_green_on_a_passing_test_passes_entry(tmp_path: Path) -> None: + """The discriminating control for the case above.""" + contracts = tmp_path / "contracts" + (tmp_path / "src").mkdir() + (tmp_path / "src" / "shipped.py").write_text("SENTINEL = 1\n") + _write_contract( + contracts, + """ + ticket_id: OMN-16824 + dod_evidence: + - id: dod-passing + description: A test_passes entry whose check_value succeeds + checks: + - check_type: test_passes + check_value: "pytest --version && grep -q SENTINEL src/shipped.py" + """, + ) + exit_code = run_compliance_check( + pr_number=1, + repo="OmniNode-ai/onex_change_control", + contracts_dir=contracts, + workspace=tmp_path, + ) + assert exit_code == 0