Skip to content

fix(OMN-20597): parse only leading quoted entries in SYNC_PATHS - #4986

Merged
jonahgabriel merged 2 commits into
devfrom
omn-20597-sync-paths-comment-quotes
Oct 15, 2026
Merged

jonahgabriel merged 2 commits into
devfrom
omn-20597-sync-paths-comment-quotes

Conversation

@jonahgabriel

Copy link
Copy Markdown
Collaborator

Summary

Linear OMN-20597 acceptance criterion AC1 (knowledge-base-internal in the mirror script list and in config/runner_fleet.yaml git_mirror.repos, with a test that fails when they differ) was already merged to dev as commit 9d0471b (#4582). This PR fixes a defect in the checker that AC2's falsifier uses.

Why

scripts/ci/check_runner_host_artifact_freshness.py parse_sync_paths took every quoted string on each SYNC_PATHS line of scripts/deploy-runners.sh, comments included. A comment quotes "/omni-curl": not found, so "/omni-curl" came back as a sync path and --mode report --runner-host omninode-pc raised RefResolutionError ("/omni-curl is not present at origin/dev") before comparing any host artifact.

Fix: _QUOTED_ENTRY_RE is anchored to the line start, so only a line's leading quoted string is an entry.

Tests

New tests in tests/ci/test_check_runner_host_artifact_freshness.py:

  • test_parse_sync_paths_ignores_quoted_text_in_comments (failed before the fix with entries /omni-curl and note extra)
  • test_live_deploy_script_sync_paths_all_exist_in_tree
  • test_recorded_deploy_script_sync_paths_block_yields_no_comment_entry, a live_contact test replaying the SYNC_PATHS block recorded byte-for-byte from scripts/deploy-runners.sh at fca1f6e (tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json)

Added contracts/OMN-20597.yaml binding AC1 and a new AC4.

Lab

Lab: host omninode-pc (lab host h201), lane omn20597-tix, command uv run pytest tests/ci/test_check_runner_host_artifact_freshness.py tests/scripts/test_deploy_runners_sync_paths.py tests/ci/test_runner_host_hook_converge_omn18819.py -q -n 0 -p no:randomly, observed 30 passed before the recorded test was added. After the fix check_runner_host_artifact_freshness.py --mode report --runner-host omninode-pc ran to completion and printed "28 path(s) stale on host", every remote hash UNREADABLE because ssh from this host to itself fails with "Host key verification failed".

Not done

Read-only checks on omninode-pc at 2026-10-15T09:33Z: the deployed docker/runners/git-mirror-refresh.sh under the host runner directory has sha256 6b592359f43c..., origin/dev has 66f596098c23..., so AC2 is not met. The deployed copy lists no knowledge-base-internal and the mirror root has no knowledge-base-internal.git (it holds omnibase_core, omnibase_infra, omniclaude, omnimarket, omninode_infra, onex_change_control). The omninode-git-mirror-refresh.timer is active. Converging the host copy was not run from this lane. AC3 (a rerun job logging a c2-mirror pre-seed line with checkout under 30 s) is not checked.

Closes OMN-20597

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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: correctness, security-contracts
Models failed: none
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 6
Nit-level findings suppressed: 0

The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.

Below quorum: 6 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [MAJOR] tests/ci/test_check_runner_host_artifact_freshness.py (correctness) — test_live_deploy_script_sync_paths_all_exist_in_tree may fail if a path is a directory | The test test_live_deploy_script_sync_paths_all_exist_in_tree asserts that every parsed path is a file via `(
  • [MAJOR] tests/ci/test_check_runner_host_artifact_freshness.py (security-contracts) — Live test depends on file existence in repo, which may not hold in all CI environments | The test test_live_deploy_script_sync_paths_all_exist_in_tree reads scripts/deploy-runners.sh from the repo
  • [MINOR] tests/ci/fixtures/ci_live_contact/deploy_runners_sync_paths_omn20597.json (correctness) — Fixture JSON provenance references a specific commit SHA but the test does not verify it | The fixture deploy_runners_sync_paths_omn20597.json includes a source_sha256 and a commit SHA in the prov
  • [MINOR] tests/ci/test_check_runner_host_artifact_freshness.py (correctness) — test_parse_sync_paths_ignores_quoted_text_in_comments does not test a comment line with no leading whitespace | The test fixture includes a comment line `' # fails with "/omni-curl": not found at t
  • [MINOR] scripts/ci/check_runner_host_artifact_freshness.py (security-contracts) — Filter for '$' in entry may incorrectly exclude valid paths containing dollar signs | The line [entry for entry in entries if entry.strip() and "$" not in entry] filters out any entry containing a d
  • [MINOR] tests/ci/test_check_runner_host_artifact_freshness.py (security-contracts) — No test for empty SYNC_PATHS array | The change modifies how entries are parsed, but there is no test for the case where the SYNC_PATHS array is empty (SYNC_PATHS=()). The regex `_SYNC_PATHS_BLOCK_R

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

✅ Hostile Reviewer — REVIEWED

Critical findings: 0
Major findings: 2
Total findings: 2
Models succeeded: correctness,security-contracts
Models unavailable: none

Action required: findings were posted as review threads. Address or reject each one, then resolve its thread — the Hostile Review Thread Gate fails while hostile-reviewer threads are unresolved (OMN-17492).


Semantics (OMN-17492 — the model finds, thread resolution gates)

Surface Meaning Blocks merge?
Review threads Per-finding, posted by the reviewer No (informational)
Hostile Review Thread Gate Deterministic: unresolved hostile-reviewer threads exist Fails until resolved (not yet a required context)
degraded verdict Fewer than 2 models succeeded (infra) Fails this job with a named reason

Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review; voters from the private CI overlay (OMN-8468/OMN-17492/OMN-20910/OMN-20923)

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.
@jonahgabriel
jonahgabriel added this pull request to the merge queue Oct 15, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hostile Reviewer — adversarial findings (OMN-17492)

Models succeeded: correctness, security-contracts
Models failed: none
New finding threads: 0
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 2
Nit-level findings suppressed: 0

The model is the FINDER, never the gate: merge is gated only by the
deterministic Hostile Review Thread Gate, which blocks while
hostile-reviewer threads are unresolved. Resolve each thread after
addressing (or rejecting, with a reply) its finding.

Below quorum: 2 finding(s) raised by one model only (OMN-18479)

These are reported and NOT dropped, but they get no thread and do not block: a single model's finding no other model reproduced is not evidence enough to stop a merge. Read them; act on them if they are right.

  • [MAJOR] scripts/ci/check_runner_host_artifact_freshness.py (security-contracts) — Parser fails on inline comments preceding a path entry | The fix anchors _QUOTED_ENTRY_RE to the start of the line (r'^\s*"([^"]*)"'). This correctly ignores comment lines that start with '#', but it
  • [MAJOR] scripts/ci/check_runner_host_artifact_freshness.py (security-contracts) — Parser fails on lines with leading whitespace before a comment | The regex r'^\s*"([^"]*)"' requires the quote to be the first non-whitespace character. If a line has leading whitespace followed by

Merged via the queue into dev with commit 2ba134c Oct 15, 2026
144 checks passed
@jonahgabriel
jonahgabriel deleted the omn-20597-sync-paths-comment-quotes branch October 15, 2026 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant