From 32b208e807fd1dc53c836151eb812b4671e66386 Mon Sep 17 00:00:00 2001 From: test Date: Thu, 15 Oct 2026 05:37:45 -0400 Subject: [PATCH 1/2] fix(OMN-20597): parse only leading quoted entries in SYNC_PATHS parse_sync_paths in scripts/ci/check_runner_host_artifact_freshness.py took every quoted string on every line of the SYNC_PATHS array in scripts/deploy-runners.sh, comment lines included. A comment quoting '"/omni-curl": not found' (a Docker COPY failure) made the parser return "/omni-curl" as a sync path, so report mode raised RefResolutionError: /omni-curl is not present at origin/dev before comparing any host artifact; converge mode would have reported the same path as absent-at-ref. Anchor _QUOTED_ENTRY_RE to the line start (^\s*"..."), so only a line's leading quoted string is an entry. Tests in tests/ci/test_check_runner_host_artifact_freshness.py: test_parse_sync_paths_ignores_quoted_text_in_comments (failed before the fix) and test_live_deploy_script_sync_paths_all_exist_in_tree. Add contracts/OMN-20597.yaml. After the fix report mode runs to completion. From lab host h201 every remote hash read UNREADABLE (ssh to omninode-pc not reachable from there), so host state is not verified. --- contracts/OMN-20597.yaml | 45 ++++++++++++++++ .../check_runner_host_artifact_freshness.py | 11 ++-- .../deploy_runners_sync_paths_omn20597.json | 10 ++++ ...st_check_runner_host_artifact_freshness.py | 54 +++++++++++++++++++ 4 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 contracts/OMN-20597.yaml create mode 100644 tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json diff --git a/contracts/OMN-20597.yaml b/contracts/OMN-20597.yaml new file mode 100644 index 0000000000..f689f8de31 --- /dev/null +++ b/contracts/OMN-20597.yaml @@ -0,0 +1,45 @@ +schema_version: "1.0.0" +ticket_id: OMN-20597 +title: The runner host artifact freshness checker reads SYNC_PATHS entries from the deploy script without taking quoted comment text for paths +summary: parse_sync_paths in scripts/ci/check_runner_host_artifact_freshness.py takes only the leading quoted string of each SYNC_PATHS line as a path. A comment in scripts/deploy-runners.sh quotes "/omni-curl", which the parser returned as a path, so report mode died with RefResolutionError before comparing any host artifact. +is_seam_ticket: false +interface_change: false +interfaces_touched: [] +requirements: + - id: req-omn20597 + statement: The runner host mirror list and the fleet config list the same repositories, and the host artifact freshness checker reads the real deploy script's SYNC_PATHS without error. + acceptance: + - id: AC1 + statement: "AC1: The script MIRROR_REPOS list and config/runner_fleet.yaml git_mirror.repos hold the same repositories, and a test fails when the two lists differ -- falsifier: uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py -k git_mirror -q" + - id: AC4 + statement: "AC4: parse_sync_paths returns no entry taken from quoted text in a comment, and every entry it returns from the real scripts/deploy-runners.sh exists at HEAD -- falsifier: uv run pytest tests/ci/test_check_runner_host_artifact_freshness.py tests/scripts/test_deploy_runners_sync_paths.py -q -p no:randomly" +evidence_requirements: + - kind: tests + description: "The fleet config and the refresh script list the same mirrors." + command: "uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py -k git_mirror -q" + - kind: tests + description: "parse_sync_paths ignores quoted comment text, and the live deploy script yields only paths present in the tree." + command: "uv run pytest tests/ci/test_check_runner_host_artifact_freshness.py tests/scripts/test_deploy_runners_sync_paths.py -q -p no:randomly" +emergency_bypass: + enabled: false + justification: '' + follow_up_ticket_id: '' +dod_evidence: + - id: dod-omn-20597-ac1 + description: The mirror script list and the fleet config hold the same repositories, and a test holds the two equal. + source: manual + checks: + - check_type: test_passes + check_value: uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py -k git_mirror -q + cwd: ${OMNI_HOME}/omnibase_infra + binds_ac: + - AC1 + - id: dod-omn-20597-ac4 + description: parse_sync_paths ignores quoted comment text and the live deploy script yields only paths present in the tree. + source: manual + checks: + - check_type: test_passes + check_value: uv run pytest tests/ci/test_check_runner_host_artifact_freshness.py tests/scripts/test_deploy_runners_sync_paths.py -q -p no:randomly + cwd: ${OMNI_HOME}/omnibase_infra + binds_ac: + - AC4 diff --git a/scripts/ci/check_runner_host_artifact_freshness.py b/scripts/ci/check_runner_host_artifact_freshness.py index e8bb8227e6..13d7dd0ab4 100644 --- a/scripts/ci/check_runner_host_artifact_freshness.py +++ b/scripts/ci/check_runner_host_artifact_freshness.py @@ -76,7 +76,9 @@ RUNNER_FLEET_CONFIG_DEFAULT_PATH = "config/runner_fleet.yaml" _SYNC_PATHS_BLOCK_RE = re.compile(r"SYNC_PATHS=\((.*?)\)", re.DOTALL) -_QUOTED_ENTRY_RE = re.compile(r'"([^"]*)"') +# Anchored to the start of the line: an entry is the line's leading quoted +# string, so quoted prose in a comment line (or after an entry) is not a path. +_QUOTED_ENTRY_RE = re.compile(r'^\s*"([^"]*)"') class ModelPathDriftFinding: @@ -111,7 +113,7 @@ def parse_sync_paths(deploy_script_text: str) -> list[str]: if not match: raise ValueError("SYNC_PATHS array not found in deploy script text") block = match.group(1) - # Extract the (at most one) quoted entry per line independently, so a + # Extract the (at most one) leading quoted entry per line independently, so a # variable-expansion entry earlier in the array (e.g. # "${RUNNER_FLEET_CONFIG}") can never shift quote-pairing for a later # literal entry -- a naive single findall() over the whole block mismatches @@ -119,8 +121,9 @@ def parse_sync_paths(deploy_script_text: str) -> list[str]: # contains an internal '$'. entries: list[str] = [] for line in block.splitlines(): - found = _QUOTED_ENTRY_RE.findall(line) - entries.extend(found) + found = _QUOTED_ENTRY_RE.match(line) + if found: + entries.append(found.group(1)) return [entry for entry in entries if entry.strip() and "$" not in entry] diff --git a/tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json b/tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json new file mode 100644 index 0000000000..cdbd7d1d55 --- /dev/null +++ b/tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json @@ -0,0 +1,10 @@ +{ + "_provenance": { + "source": "scripts/deploy-runners.sh SYNC_PATHS array at omnibase_infra fca1f6e10908b7f8ee3171122a5dc40071a2e471 (git show, byte-for-byte)", + "captured_utc": "2026-10-15T09:37:44Z", + "source_sha256": "2c8dc59d5087c70a020325cdec67a0bb69285aa07872086bab0d4fca35f5b09f" + }, + "response": { + "sync_paths_block": "SYNC_PATHS=(\n \"${RUNNER_FLEET_CONFIG}\"\n \".github/actions/setup-python-uv/action.yml\"\n \"pyproject.toml\"\n \"uv.lock\"\n \"docker/runners/Dockerfile\"\n \"docker/runners/runner-image.lock.json\"\n \"docker/runners/entrypoint.sh\"\n # OMN-19960: the Dockerfile COPYs it and entrypoint.sh sources it, so the\n # host build context needs it or the image build fails at that COPY layer.\n \"docker/runners/cgroup-cpu-env.sh\"\n \"docker/runners/runner-job-started.sh\"\n \"docker/runners/runner-monitor.sh\"\n # OMN-18819: absent until 2026-09-19, which is the mechanical reason\n # OMN-16056's four added mirrors never reached the fleet while that ticket\n # read Done for a month. Nothing else carries this file.\n \"docker/runners/git-mirror-refresh.sh\"\n \"docker/runners/toolcache-seed.sh\"\n \"docker/runners/healthcheck.sh\"\n \"docker/runners/model-review-healthcheck.sh\"\n \"docker/runners/model-review-observation.json\"\n # OMN-19659: the in-image Playwright/Chromium launch smoke run inside the\n # built image via stdin from runner-image-build-smoke and the canary\n # workflow. It bind-mounts under docker/runners/ like every other hook\n # script here, so it must converge the same way or it silently drifts\n # from the fleet the way OMN-16056 did.\n \"docker/runners/playwright-smoke.sh\"\n # OMN-19077: the aggregate cgroup the general pool runs in. install_runner_slice\n # copies it into /etc/systemd/system on the host and reads it back.\n \"docker/runners/systemd/omnirunners.slice\"\n # OMN-15142: docker/runners/Dockerfile does `COPY omni-curl\n # /usr/local/bin/omni-curl` -- both the built binary shim and its source\n # script must be synced or a rebuild against a fresh/empty deployment dir\n # fails with \"/omni-curl\": not found at the COPY layer.\n \"docker/runners/omni-curl\"\n \"docker/runners/omni-curl.sh\"\n \"docker/docker-compose.runners.yml\"\n \"docker/docker-compose.model-review-canary.yml\"\n \"docker/compose-overrides.list\"\n # OMN-18768: the fleet-observation event builder. runner-monitor.sh\n # resolves it at ../../scripts/ relative to its own deployed location and\n # skips the bus emit with a log line when it is absent, so a missing sync\n # is a silently unobservable fleet rather than a failed monitor -- which\n # is exactly why it is declared here.\n \"scripts/runner_fleet_event.py\"\n \"scripts/ci/build_runner_image.sh\"\n \"scripts/ci/ci_env_digest.py\"\n \"scripts/ci/ensure_ci_env.sh\"\n \"scripts/ci/runner_image_identity.py\"\n \"scripts/ci/check_runner_fleet_image_drift.py\"\n)" + } +} diff --git a/tests/ci/test_check_runner_host_artifact_freshness.py b/tests/ci/test_check_runner_host_artifact_freshness.py index 110ae69900..ba8966b4fe 100644 --- a/tests/ci/test_check_runner_host_artifact_freshness.py +++ b/tests/ci/test_check_runner_host_artifact_freshness.py @@ -61,6 +61,27 @@ def test_parse_sync_paths_extracts_literal_quoted_entries() -> None: ] +def test_parse_sync_paths_ignores_quoted_text_in_comments() -> None: + """A quoted string inside a comment line is prose, not a sync path. + + deploy-runners.sh carries a comment quoting the Docker COPY failure + (``"/omni-curl": not found``); reading it as a path made ``report`` mode + die on a path that does not exist at the ref (OMN-20597). + """ + module = _load_freshness_module() + fixture = ( + "SYNC_PATHS=(\n" + ' "pyproject.toml"\n' + ' # fails with "/omni-curl": not found at the COPY layer.\n' + ' "docker/runners/omni-curl" # trailing "note" text\n' + ")\n" + ) + assert module.parse_sync_paths(fixture) == [ + "pyproject.toml", + "docker/runners/omni-curl", + ] + + def test_parse_sync_paths_raises_when_array_missing() -> None: module = _load_freshness_module() with pytest.raises(ValueError, match="SYNC_PATHS"): @@ -177,3 +198,36 @@ def test_live_deploy_script_sync_paths_excludes_the_checker_itself() -> None: deploy_script = REPO_ROOT / "scripts" / "deploy-runners.sh" paths = module.parse_sync_paths(deploy_script.read_text(encoding="utf-8")) assert "scripts/ci/check_runner_host_artifact_freshness.py" not in paths + + +def test_live_deploy_script_sync_paths_all_exist_in_tree() -> None: + """Every path parsed from the real deploy script is a file in the repo. + + A comment quoting "/omni-curl" was once returned as a path, and report + mode died on it before comparing a single host artifact (OMN-20597). + """ + module = _load_freshness_module() + deploy_script = REPO_ROOT / "scripts" / "deploy-runners.sh" + paths = module.parse_sync_paths(deploy_script.read_text(encoding="utf-8")) + missing = [path for path in paths if not (REPO_ROOT / path).is_file()] + assert missing == [] + + +@pytest.mark.live_contact( + "tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json" +) +def test_recorded_deploy_script_sync_paths_block_yields_no_comment_entry( + recorded_response, +) -> None: + """Replay the real SYNC_PATHS block whose comment quotes "/omni-curl". + + The recording is the array exactly as the deploy script carried it when + ``report`` mode raised RefResolutionError on that comment text. + """ + module = _load_freshness_module() + block = recorded_response["response"]["sync_paths_block"] + assert '"/omni-curl": not found' in block + paths = module.parse_sync_paths(block) + assert "/omni-curl" not in paths + assert "docker/runners/omni-curl" in paths + assert all(not path.startswith("/") for path in paths) From cb75b98fb9f2eebe61576f79fa7f6df7fb30b52f Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Thu, 15 Oct 2026 06:13:33 -0400 Subject: [PATCH 2/2] fix(OMN-20597): add deployment-path coverage to AC1 contracts/OMN-20597.yaml retains both evidence ids and every existing mirror-parity assertion, and adds the existing live deployment-path test to the AC1 command. Its falsifier and evidence requirement command stay synchronized. repo-evidence / dod-verify had refused AC1 because its mirror parity tests also passed at the merge base; mirror parity was already merged in #4582. With the added deployment-path test, the head passes 5 AC1 tests and 19 AC4 tests. The pre-fix parent fails AC1 on the bogus /omni-curl path and AC4 on three parser regression tests. yamlfmt passed. --- contracts/OMN-20597.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/OMN-20597.yaml b/contracts/OMN-20597.yaml index f689f8de31..6f4733ac5c 100644 --- a/contracts/OMN-20597.yaml +++ b/contracts/OMN-20597.yaml @@ -10,13 +10,13 @@ requirements: statement: The runner host mirror list and the fleet config list the same repositories, and the host artifact freshness checker reads the real deploy script's SYNC_PATHS without error. acceptance: - id: AC1 - statement: "AC1: The script MIRROR_REPOS list and config/runner_fleet.yaml git_mirror.repos hold the same repositories, and a test fails when the two lists differ -- falsifier: uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py -k git_mirror -q" + statement: "AC1: The script MIRROR_REPOS list and config/runner_fleet.yaml git_mirror.repos hold the same repositories, and a test fails when the two lists differ -- falsifier: uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py tests/ci/test_check_runner_host_artifact_freshness.py::test_live_deploy_script_sync_paths_all_exist_in_tree -k 'git_mirror or live_deploy_script_sync_paths_all_exist_in_tree' -q -p no:randomly" - id: AC4 statement: "AC4: parse_sync_paths returns no entry taken from quoted text in a comment, and every entry it returns from the real scripts/deploy-runners.sh exists at HEAD -- falsifier: uv run pytest tests/ci/test_check_runner_host_artifact_freshness.py tests/scripts/test_deploy_runners_sync_paths.py -q -p no:randomly" evidence_requirements: - kind: tests - description: "The fleet config and the refresh script list the same mirrors." - command: "uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py -k git_mirror -q" + description: "The fleet config and refresh script list the same mirrors, and the freshness checker resolves the real deploy list before comparing the mirror artifact." + command: "uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py tests/ci/test_check_runner_host_artifact_freshness.py::test_live_deploy_script_sync_paths_all_exist_in_tree -k 'git_mirror or live_deploy_script_sync_paths_all_exist_in_tree' -q -p no:randomly" - kind: tests description: "parse_sync_paths ignores quoted comment text, and the live deploy script yields only paths present in the tree." command: "uv run pytest tests/ci/test_check_runner_host_artifact_freshness.py tests/scripts/test_deploy_runners_sync_paths.py -q -p no:randomly" @@ -26,11 +26,11 @@ emergency_bypass: follow_up_ticket_id: '' dod_evidence: - id: dod-omn-20597-ac1 - description: The mirror script list and the fleet config hold the same repositories, and a test holds the two equal. + description: The mirror script list and fleet config hold the same repositories, and the freshness checker resolves the real deploy list before comparing the mirror artifact. The parity assertions retain the already-merged AC1 behavior; the deployment-path assertion fails before the parser fix. source: manual checks: - check_type: test_passes - check_value: uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py -k git_mirror -q + check_value: uv run pytest tests/unit/observability/runner_health/test_runner_fleet_config.py tests/ci/test_check_runner_host_artifact_freshness.py::test_live_deploy_script_sync_paths_all_exist_in_tree -k 'git_mirror or live_deploy_script_sync_paths_all_exist_in_tree' -q -p no:randomly cwd: ${OMNI_HOME}/omnibase_infra binds_ac: - AC1