Repository navigation
fix(OMN-20798): lane census fails loud when it cannot publish drift; nightly sweep unit waits for its broker - #4780
Conversation
scripts/lane-census-check.sh now exits 8 (EXIT_DRIFT_UNPUBLISHED) when a drift event is not published; previously it exited 30, the same code as a published drift. The drift event is produced through the broker container named by LANE_MEMORY_BROKER_CONTAINER using the broker_produce helper, with the SASL pair named by variable inside the container. The KAFKA_BOOTSTRAP_SERVERS and host-side rpk branch are removed because rpk is not on a lab host PATH. With no broker container named, the run logs "DRIFT event NOT published" and exits 8. A memory-pass code no longer replaces exit 8; it is logged beside it. New test tests/unit/scripts/test_lane_census_unpublishable_drift_omn20798.py (5 tests); tests/unit/scripts/test_lane_census_dry_run.py updated for the new contract. Unit comments in deploy/lane-census mention exit 8.
…er health gate scripts/systemd/onex-nightly-sweep.service now carries the whole publish mechanism; the installed unit on the .201 host had run ~/.local/bin/onex-nightly-sweep-trigger.sh, a file no repo tracked. The old run failed 2026-10-09 with "OCI runtime exec failed ... setns process: exit status 1"; the 2026-10-07 03:00 run had hung until 2026-10-09 10:01 with no timeout; the dev broker now requires SASL. ExecStartPre waits up to 120 seconds for the broker container to report healthy and otherwise fails the unit before publishing. TimeoutStartSec=300 bounds the start. The SASL pair is expanded inside the broker container by variable name, never by value on the host. Each run uses a fresh correlation id (the old script sent one fixed id every night). The topic stays onex.cmd.omnibase-infra.build-loop-start.v1. New tests/unit/scripts/test_onex_nightly_sweep_unit_omn20798.py (5 tests) parse the unit the way systemd does and run its commands against a docker shim. Lab: on the .201 host the unit was installed from this file and started; the journal shows "Produced to partition 0 at offset 41" and the unit finished with Result=success.
…13455) * evidence(OMN-20798): author OCC companion for OmniNode-ai/omnibase_infra#4780 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 9c41170fac80ca266b497084da8df9d3d75efb2b. * evidence(OMN-20798): self-bind OCC#13455 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: qwen3-review, local-studio-planner
Models failed: none
New finding threads: 4
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 8
Nit-level findings suppressed: 1
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: 8 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.
- [CRITICAL]
scripts/systemd/onex-nightly-sweep.service(local-studio-planner) — Command injection via broker container name in ExecStartPre | TheExecStartPrecommand inonex-nightly-sweep.servicepasses the broker container name as a positional argument to a bash script: `.. - [MAJOR]
scripts/lane-census-check.sh(qwen3-review) — Shell injection via unquoted variable expansion in broker_produce | Thebroker_producefunction constructs a shell command string fordocker execby interpolating$user_var,$pass_var, `$mecha - [MAJOR]
scripts/lane-census-check.sh(qwen3-review) — Incorrect exit code precedence in finish() function | Thefinish()function inlane-census-check.shgives precedence toMEMORY_RCover the census exit coderc, except whenrcis `EXIT_DRIFT_ - [MAJOR]
scripts/systemd/onex-nightly-sweep.service(local-studio-planner) — Race condition in health check loop | TheExecStartPreloop inonex-nightly-sweep.servicepolls the container health status every 2 seconds. If the container becomes healthy between checks, it pro - [MAJOR]
tests/unit/scripts/test_onex_nightly_sweep_unit_omn20798.py(local-studio-planner) — Test shim does not validate SASL variable expansion safety | The testtest_onex_nightly_sweep_unit_omn20798.pyshimsdockerand checks the command line arguments. It verifies that `${DEV_KAFKA_SAS - [MINOR]
tests/unit/scripts/test_lane_census_unpublishable_drift_omn20798.py(qwen3-review) — Missing test for concurrent execution of lane-census-check.sh | The tests forlane-census-check.shdo not cover concurrent execution. If two instances of the script run simultaneously, they may inte - [MINOR]
scripts/lane-census-check.sh(qwen3-review) — Missing error handling for docker exec failure in broker_produce | Thebroker_producefunction does not handle the case wheredocker execfails due to a non-existent container or other Docker erro - [MINOR]
scripts/lane-census-check.sh(local-studio-planner) — Ambiguous exit code handling in finish function | Inlane-census-check.sh, thefinishfunction checks ifMEMORY_RCis non-zero. If it is, it logs and exits withMEMORY_RC. However, ifrc(th
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (qwen3-review)
Shell injection in onex-nightly-sweep.service ExecStart | The
ExecStartdirective inonex-nightly-sweep.serviceuses$$1to pass the broker container name into abash -cscript. The variable$$1is expanded by systemd, but the resulting value is then used in adocker execcommand inside the bash script. If theONEX_SWEEP_BROKER_CONTAINERenvironment variable is set to a value containing shell metacharacters, it could lead to command injection. The use ofsh -cinsidedocker execwith unquoResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (local-studio-planner)
Credential exposure in docker exec command | In
onex-nightly-sweep.service, the SASL credentials are passed via environment variables to the docker container. While the comment claims they are not on argv, thedocker execcommand constructs a shell command string that includes variable expansion:sh -c 'RPK_USER="${DEV_KAFKA_SASL_USERNAME}" ...'. If${DEV_KAFKA_SASL_USERNAME}or${DEV_KAFKA_SASL_PASSWORD}contain single quotes or other shell metacharacters, they could break out of the string literResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (qwen3-review)
Inefficient health check loop in onex-nightly-sweep.service | The
ExecStartPredirective inonex-nightly-sweep.servicepolls the broker container's health status every 2 seconds for up to 120 seconds. This is inefficient and could delay the start of the service unnecessarily. A more efficient approach would be to use a singledocker waitcommand or a similar mechanism to wait for the container to become healthy. | Evidence: ExecStartPre=/bin/bash -c 'for i in$$(seq 1 60); do if [ "$$ (docker inspect -Resolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MINOR] hostile-reviewer (local-studio-planner)
Inefficient health check polling interval | The health check loop in
ExecStartPresleeps for 2 seconds between checks. With a maximum of 60 iterations, this allows up to 120 seconds (2 minutes) for the container to become healthy. This is consistent with the comment but may be too slow if the container typically starts faster. Conversely, if it fails, the unit waits the full duration. | Evidence: ExecStartPre=/bin/bash -c 'for i in $$(seq 1 60); do ... sleep 2; done' | Fix: Consider reducing the sleep intResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
✅ Hostile Reviewer — REVIEWEDCritical findings: 3
Semantics (OMN-17492 — the model finds, thread resolution gates)
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review: qwen3-review (Qwen3.8-27B), local-studio-planner (Qwen3.6-35B-A3B) (OMN-8468/OMN-8524/OMN-17492) |
Adds contracts/OMN-20798.yaml to omnibase_infra so Repo Evidence Gate (repo-evidence / dod-verify) finds a contract at the PR head; it failed with 'pull request cites OMN-20798 but carries no contracts/OMN-20798.yaml'. AC1 binds to the census unpublishable-drift and dry-run tests; AC2 binds to the nightly sweep unit tests.
There was a problem hiding this comment.
Hostile Reviewer — adversarial findings (OMN-17492)
Models succeeded: qwen3-review, local-studio-planner
Models failed: none
New finding threads: 2
Deduped (already posted on this PR): 0
Below quorum (one model only, reported not threaded): 10
Nit-level findings suppressed: 1
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: 10 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.
- [CRITICAL]
scripts/systemd/onex-nightly-sweep.service(qwen3-review) — Shell injection via unquoted variable in docker exec command | Thebroker_producefunction constructs a shell command string where the$topicvariable is interpolated without proper quoting. The l - [CRITICAL]
scripts/systemd/onex-nightly-sweep.service(qwen3-review) — ExecStartPre uses wrong argument index for container name | TheExecStartPredirective invokes/bin/bash -c '...' sweep ${ONEX_SWEEP_BROKER_CONTAINER}. Inside the single-quoted script,$$1expan - [CRITICAL]
scripts/lane-census-check.sh; scripts/systemd/onex-nightly-sweep.service(local-studio-planner) — Credential Injection via Broker Container Name | Thebroker_producefunction inlane-census-check.shand theExecStartcommand inonex-nightly-sweep.serviceconstruct shell commands by interpo - [MAJOR]
tests/unit/scripts/test_onex_nightly_sweep_unit_omn20798.py(qwen3-review) — Test for nightly sweep unit will fail due to argument index bug | The testtest_a_healthy_broker_gets_one_command_per_run_with_a_fresh_idruns theExecStartcommand and expects it to succeed. Howe - [MAJOR]
scripts/systemd/onex-nightly-sweep.service(local-studio-planner) — Insecure Credential Expansion in Nightly Sweep | The nightly sweep unit expands credentials using${DEV_KAFKA_SASL_USERNAME}and${DEV_KAFKA_SASL_PASSWORD}inside thesh -ccommand. While these - [MAJOR]
tests/unit/scripts/test_lane_census_unpublishable_drift_omn20798.py(local-studio-planner) — Incomplete Test Coverage for Error Paths | The testtest_lane_census_unpublishable_drift_fails_when_the_produce_failsmocks thedocker execcommand to return a non-zero exit code. However, it does - [MINOR]
scripts/lane-census-check.sh(qwen3-review) — SASL credentials exposed in process environment | Thebroker_producefunction passes SASL credentials via environment variables to thedocker execcommand. While the credentials are expanded insid - [MINOR]
tests/unit/scripts/test_onex_nightly_sweep_unit_omn20798.py(qwen3-review) — Missing test for ExecStartPre argument index bug | The test suite does not include a test that would catch the argument index bug in the systemd unit file. The test `test_an_unhealthy_broker_fails_the - [MINOR]
scripts/systemd/onex-nightly-sweep.service(local-studio-planner) — Inefficient Broker Health Check Polling | The health check loop inonex-nightly-sweep.serviceuses a fixed sleep interval of 2 seconds. This may be too aggressive for some environments, causing unne - [MINOR]
scripts/lane-census-check.sh(local-studio-planner) — Tight Coupling Between Script and Broker Container | Thelane-census-check.shscript is tightly coupled to the broker container's internal structure (e.g., expectingrpkto be available inside the
Findings demoted from threads (anchor rejected)
-
[MAJOR] hostile-reviewer (qwen3-review)
ExecStart uses wrong argument index for container name | The
ExecStartdirective has the same issue: it passessweepas the first argument and${ONEX_SWEEP_BROKER_CONTAINER}as the second. The script body uses$$1to reference the container name indocker exec -i "$$1", which will resolve tosweepinstead of the actual container name. This will cause the produce command to fail because it will try to exec into a non-existent container namedsweep. | Evidence: ExecStart=/bin/bash -c 'set -euo pResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492). -
[MAJOR] hostile-reviewer (local-studio-planner)
Race Condition in Broker Health Check | The
ExecStartPrecommand inonex-nightly-sweep.servicepolls the broker container's health status every 2 seconds for up to 120 seconds. However, if the container becomes healthy between checks, the script will proceed. If the container becomes unhealthy immediately after the check passes but before theExecStartcommand runs, the produce operation may fail. This is a standard TOCTOU race condition in health checking. While bounded, it does not guarantee that thResolve this thread when addressed — the
Hostile Review Thread Gateblocks while hostile-reviewer threads are unresolved (OMN-17492).
lane-census-check.sh now exits 8 when a drift event is not published, while a published drift still exits 30; previously both cases exited 30. The drift event is produced through the broker container named by LANE_MEMORY_BROKER_CONTAINER, which the installer writes into the unit, using the same helper as the memory pass. The KAFKA_BOOTSTRAP_SERVERS plus host-side rpk branch is removed because rpk is not on a lab host PATH.
scripts/systemd/onex-nightly-sweep.service now carries the whole publish mechanism instead of running an untracked host script. It waits up to 120 s for the broker container to be healthy and fails before publishing if it is not, has TimeoutStartSec=300, names the SASL pair by variable inside the container, and sends a fresh correlation id per run. The previous nightly run had failed with "OCI runtime exec failed ... setns process: exit status 1"; the run before it had hung from 2026-10-07 03:00 to 2026-10-09 10:01. Not done in this PR: the second lab host (h202) could not be reached from this lane; h202 is out of lab placement per the operator's 2026-10-09 ruling under OMN-20769 and ssh to it from the lane host fails host key verification.
Acceptance criteria (OMN-20798)
uv run pytest tests -q -k lane_census_unpublishable_drift_failson h201 at head 9c41170: the three*_fails_*tests were red before the change (exit 30 where 8 is expected) and are green after; the published-drift and dry-run companions intests/unit/scripts/test_lane_census_unpublishable_drift_omn20798.pyare green.test_lane_census_dry_run.pywas updated to the new contract (no broker container is now exit 8, not 30).onex-nightly-sweep.service: MET. Unit installed from this branch's file,systemctl --user start onex-nightly-sweep.servicereturned 0,Result=success ExecMainStatus=0.journalctl --user -u onex-nightly-sweep -n 5:Starting onex-nightly-sweep.service - ONEX Nightly Sweep — publishes build-loop-start to the dev lane broker...onex-nightly-sweep[2324809]: Produced to partition 0 at offset 41 with timestamp 1791568984154.onex-nightly-sweep[2324775]: onex-nightly-sweep: build-loop-start bcaca6ae-c011-4167-bb22-f9257ebffa77 published via omnibase-infra-redpandaFinished onex-nightly-sweep.service - ONEX Nightly Sweep — publishes build-loop-start to the dev lane broker.onex-lane-census.service: NOT PROVEN by this lane. h202 is out of lab placement (operator ruling 2026-10-09 under OMN-20769, soonex-lab-run --host h202refuses) and ssh to it from the h201 lane host fails host key verification. The host steps are below.h202 steps (for the operator or the launching host)
From the refreshed omnibase_infra clone on h202 at this PR's merge:
bash deploy/lane-census/install-lane-census.sh --standalone --broker-container omnibase-infra-dev-202-redpanda --repo-root /data/omninode/omnibase_infra, thensystemctl --user start onex-lane-census.serviceandjournalctl --user -u onex-lane-census -n 5. Expected: with the broker container named, a drift is published (published lane-census-drift event ... via broker container) and the unit exits 30 only while a declared container is genuinely absent (the dev-202 Phoenix of the 2026-10-09 journal); with none named it exits 8 and saysDRIFT event NOT published.Notes
onex-disk-gccensus drop-in on h201 is an older install (no--memory, no broker container) and its ExecStart carries the-prefix, so a census exit there, 8 included, does not fail that unit. Left unchanged here; reinstalling withinstall-lane-census.sh --broker-container omnibase-infra-redpandaand the fail-soft prefix are separate decisions.~/.local/bin/onex-nightly-sweep-trigger.shon h201 is no longer referenced by the unit; it is left in place and can be deleted.Lab: host=h201 (omninode-pc), lane=mon-reds-infra-r-5d21, command=
cp scripts/systemd/onex-nightly-sweep.service ~/.config/systemd/user/ && systemctl --user daemon-reload && systemctl --user start onex-nightly-sweep.servicethenjournalctl --user -u onex-nightly-sweep -n 5: unit finished Result=success and the broker answeredProduced to partition 0 at offset 41; before the change the same unit failedOCI runtime exec failed ... setns process. Census exit codes exercised live on h201 withLANE_MANIFEST=<manifest plus one absent container> bash scripts/lane-census-check.sh --lane judge: no broker container named gave exit=8 andDRIFT event NOT published: LANE_MEMORY_BROKER_CONTAINER is unset; a nonexistent broker container gave exit=8 andproduce ... FAILED; nothing reached the bus. h202 not reachable from this lane (see above). head=9c41170fac80ca266b497084da8df9d3d75efb2bDelegation: commit messages and the summary paragraph were drafted through
onex delegatevia landing_text.py and checked against the facts.Evidence-Ticket: OMN-20798
Evidence-Source: OCC#13455