From 5d5abc052fafe7924b4d344dda9756ff4b6680b5 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Fri, 28 Aug 2026 11:34:40 -0400 Subject: [PATCH] fix(OMN-16845): export OMNI_HOME in evidence-autoclose-sweep.yml so cwd-anchored dod_verify checks resolve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job never exported OMNI_HOME, so EvidenceCollector._resolve_cwd rendered every `cwd: "${OMNI_HOME}/omnibase_infra"` check to `/omnibase_infra` (which never exists) and recorded it FAILED instead of verified. Proven by execution both directions: `env -u OMNI_HOME` against the job's env shape reproduces `failed: 3, behavior_proving_count: 0`; the same run with OMNI_HOME set reproduces `verified, failed: 0, behavior_proving_count: 3`. GitHub Actions checks a repo out to `//` (the documented github.workspace convention), so github.workspace's own parent directory already satisfies `${OMNI_HOME}/omnibase_infra == github.workspace` with the real checkout — no symlink (which _resolve_cwd's containment check would reject anyway) and no second checkout. A new step derives and exports that path via GITHUB_ENV, fail-fast: it asserts the derived path actually places omnibase_infra at github.workspace before exporting, erroring loudly instead of exporting an unverified guess. Adds a workflow-lint test (mirroring test_omn_16792_kill_switch_wiring.py) asserting a step exports OMNI_HOME to $GITHUB_ENV before every dod_verify / evidence_autoclose_sweep invocation, and that the export step fails fast rather than silently defaulting. Confirmed RED against the unfixed workflow, GREEN after. OMN-16845 --- .../workflows/evidence-autoclose-sweep.yml | 50 ++++++++ .../test_omn_16845_omni_home_wiring.py | 117 ++++++++++++++++++ 2 files changed, 167 insertions(+) create mode 100644 tests/unit/nodes/node_evidence_autoclose_sweep_effect/test_omn_16845_omni_home_wiring.py diff --git a/.github/workflows/evidence-autoclose-sweep.yml b/.github/workflows/evidence-autoclose-sweep.yml index 04a17e0dc2..44fdaa026f 100644 --- a/.github/workflows/evidence-autoclose-sweep.yml +++ b/.github/workflows/evidence-autoclose-sweep.yml @@ -158,6 +158,56 @@ jobs: with: fetch-depth: 1 + # OMN-16845. `EvidenceCollector._resolve_cwd` (omnimarket) renders + # `${OMNI_HOME}` from the process environment and requires the result to + # exist; this job never exported it, so every check pinned + # `cwd: "${OMNI_HOME}/omnibase_infra"` rendered to `/omnibase_infra`, + # which never exists, and was recorded FAILED rather than skipped or + # unresolved. Proven by execution: `env -u OMNI_HOME` against this + # job's env shape reproduces `failed: 3, behavior_proving_count: 0`; + # the identical run with OMNI_HOME set reproduces + # `verified, failed: 0, behavior_proving_count: 3` — the only variable + # is the env var. + # + # GitHub Actions checks a repo out to + # `//` (the documented `github.workspace` + # convention — GitHub's own docs give `/home/runner/work/my-repo-name/ + # my-repo-name` as the canonical example), so `github.workspace`'s OWN + # PARENT directory already satisfies + # `${OMNI_HOME}/omnibase_infra == github.workspace` exactly, using the + # real checkout with no symlink and no second checkout. + # + # A symlink farm was considered and rejected: `_resolve_cwd`'s + # containment check (`candidate.is_relative_to(OMNI_HOME)`, evaluated + # AFTER `Path.resolve()`) follows a symlink to its real target and + # rejects it if that target sits outside OMNI_HOME. A fabricated + # OMNI_HOME containing a symlink pointing at the real checkout (which + # would live outside it) would fail that check. Pointing OMNI_HOME at + # the checkout's real parent is the only shape that is both correct and + # symlink-free. + # + # Fail-fast, not a silent default (CLAUDE.md rule 8): assert the + # derived path actually places omnibase_infra at github.workspace + # before exporting it, so a runner whose workspace layout ever stops + # matching that convention breaks the job loudly here instead of + # silently reintroducing this defect. + - name: Export OMNI_HOME for cwd-anchored dod_verify checks + shell: bash + run: | + set -euo pipefail + omni_home="$(dirname "${GITHUB_WORKSPACE}")" + expected="${omni_home}/omnibase_infra" + if [ "${expected}" != "${GITHUB_WORKSPACE}" ]; then + echo "::error::derived OMNI_HOME (${omni_home}) does not place omnibase_infra at github.workspace (${GITHUB_WORKSPACE}) -- the runner's workspace layout does not match the documented // convention this fix depends on." >&2 + exit 1 + fi + if [ ! -d "${expected}" ]; then + echo "::error::${expected} does not exist -- refusing to export an OMNI_HOME that dod_verify's cwd-anchored checks would immediately fail resolution against." >&2 + exit 1 + fi + echo "OMNI_HOME=${omni_home}" >> "$GITHUB_ENV" + echo "OMNI_HOME resolved to ${omni_home} (omnibase_infra -> ${expected})" + - name: Set up Python uses: actions/setup-python@v7 with: diff --git a/tests/unit/nodes/node_evidence_autoclose_sweep_effect/test_omn_16845_omni_home_wiring.py b/tests/unit/nodes/node_evidence_autoclose_sweep_effect/test_omn_16845_omni_home_wiring.py new file mode 100644 index 0000000000..ae7bf39644 --- /dev/null +++ b/tests/unit/nodes/node_evidence_autoclose_sweep_effect/test_omn_16845_omni_home_wiring.py @@ -0,0 +1,117 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""OMN-16845 — the sweep job must export a usable ``OMNI_HOME``. + +``EvidenceCollector._resolve_cwd`` (omnimarket) substitutes ``${OMNI_HOME}`` +from the process environment and requires the rendered path to exist. This +job never exported it, so every check pinned ``cwd: "${OMNI_HOME}/"`` +rendered to ``/`` and was recorded FAILED — proven by execution: +``env -u OMNI_HOME`` against this job's env shape reproduces +``failed: 3, behavior_proving_count: 0``; the identical run with a real +``OMNI_HOME`` reproduces ``verified, failed: 0, behavior_proving_count: 3`` +(see OMN-16845). + +This is a wiring assertion, not a behaviour assertion, for the same reason +``test_omn_16792_kill_switch_wiring.py`` is: the failure it guards ("the +checker reads an env var this job never sets") is invisible to any +handler-level test and only shows up by parsing the committed workflow, +which IS the wiring. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest +import yaml + +pytestmark = pytest.mark.unit + +_WORKFLOW = ( + Path(__file__).resolve().parents[4] + / ".github" + / "workflows" + / "evidence-autoclose-sweep.yml" +) + + +def _steps() -> list[dict[str, Any]]: + workflow = yaml.safe_load(_WORKFLOW.read_text(encoding="utf-8")) + return list(workflow["jobs"]["evidence-autoclose-sweep"]["steps"]) + + +def _dod_verify_invoking_indices(steps: list[dict[str, Any]]) -> list[int]: + """Steps that spawn ``uv run onex skill dod_verify`` (directly or via the + sweep, which shells out to it per ticket) — found by content, not name, + so an insertion or rename cannot silently retarget this test. + """ + indices = [] + for i, step in enumerate(steps): + run = str(step.get("run", "")) + if ( + "onex skill dod_verify" in run + or "onex skill evidence_autoclose_sweep" in run + ): + indices.append(i) + return indices + + +def _omni_home_export_index(steps: list[dict[str, Any]]) -> int | None: + """Index of the step that writes ``OMNI_HOME=`` into ``$GITHUB_ENV``. + + Must be an actual env-file export (persists to every later step), + not a step-local ``env:`` value — a step-local value would not reach + the per-ticket ``uv run onex skill dod_verify`` subprocess the sweep + step spawns internally. + """ + for i, step in enumerate(steps): + run = str(step.get("run", "")) + if "OMNI_HOME=" in run and "GITHUB_ENV" in run: + return i + return None + + +def test_a_step_exports_omni_home_to_github_env() -> None: + steps = _steps() + index = _omni_home_export_index(steps) + assert index is not None, ( + "no step in evidence-autoclose-sweep.yml writes OMNI_HOME to " + "$GITHUB_ENV — EvidenceCollector._resolve_cwd will render " + "'${OMNI_HOME}/' to '/', which never exists, and every " + "cwd-anchored behavior check will be recorded FAILED (OMN-16845)" + ) + + +def test_omni_home_is_exported_before_every_dod_verify_invocation() -> None: + steps = _steps() + export_index = _omni_home_export_index(steps) + assert export_index is not None + invoking_indices = _dod_verify_invoking_indices(steps) + assert invoking_indices, ( + "expected at least one step invoking dod_verify or the sweep — " + "found none, so this test is not actually anchored to the job" + ) + for i in invoking_indices: + assert export_index < i, ( + f"step {i} ({steps[i].get('name')!r}) runs dod_verify/the sweep " + f"before OMNI_HOME is exported at step {export_index} " + f"({steps[export_index].get('name')!r}) — GITHUB_ENV writes only " + "take effect for steps that run AFTER the write" + ) + + +def test_omni_home_export_is_fail_fast_not_a_silent_default() -> None: + """The export step must assert its own precondition rather than writing + a best-guess path — a silent default here would just move the bug from + 'never set' to 'set wrong and never checked' (CLAUDE.md rule 8).""" + steps = _steps() + index = _omni_home_export_index(steps) + assert index is not None + run = str(steps[index].get("run", "")) + assert "exit 1" in run and ("::error::" in run), ( + "the OMNI_HOME export step has no fail-fast assertion — it must " + "verify the derived path actually places the expected repo " + "checkout under it before exporting, and error loudly (exit 1) if " + "not, rather than exporting an unverified guess" + )