Skip to content

fix(OMN-15173): fail fast on unset dev-lane Redpanda advertise host - #2495

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-15173-dev-advertise-host-failfast
Jul 27, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-15173-dev-advertise-host-failfast

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jul 27, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

OMN-15173 (child of epic OMN-15154): docker-compose.infra.yml silently defaulted the dev-lane Redpanda advertise host to localhost via ${DEV_REDPANDA_ADVERTISE_HOST:-localhost} whenever the env var was unset. The dev lane runs on .201 as a shared host (deploy-runtime.sh: "Dev lane: infra.yml alone", no overlay), so an unset var silently advertised a broker address unreachable by any off-host client (CI runner, another machine) while looking healthy from .201 itself — a latent off-host regression flagged by the steel-ONEX bus-ingress scout (2026-07-26).

Seam deliverables

  • docker/docker-compose.infra.yml: both --advertise-kafka-addr and --advertise-pandaproxy-addr now use the Compose fail-fast form ${DEV_REDPANDA_ADVERTISE_HOST:?message}, matching the existing PROD_REDPANDA_ADVERTISE_HOST precedent in docker-compose.prod.yml and the check_required_env_vars.py / check-required-env-vars pre-commit gate already wired against this exact file.
  • tests/integration/docker/test_docker_integration.py: added DEV_REDPANDA_ADVERTISE_HOST to the test_compose_config_valid fixture dict — required to keep the existing CI-gate twin tests/ci/test_compose_required_env_coverage.py::test_all_required_compose_vars_in_fixture green (every :?-required compose var must appear in that fixture).
  • tests/unit/infra/test_runtime_lane_kafka_environment.py: new static regression test test_dev_redpanda_advertise_host_fails_fast_when_unset — proven RED against the pre-fix compose file (asserts the silent ${VAR:-localhost} form is absent — fails on the old file) and GREEN after the fix (asserts the :? required form is present). No docker required.
  • tests/integration/infra/test_dev_runtime_compose_render.py (new): non-mutating docker compose config render checks — unset DEV_REDPANDA_ADVERTISE_HOST fails the render loudly; an explicitly-set value is honored verbatim (never silently overridden with localhost). Skipped where docker compose is unavailable, matching the sibling test_prod_runtime_compose_render.py / test_stability_test_runtime_compose_render.py / test_judge_compose_render.py convention. Never brings up, restarts, or mutates any lane.
  • docker/README.md / docker/env-example-full.txt: moved DEV_REDPANDA_ADVERTISE_HOST into the required-vars documentation; noted the no-silent-default behavior.

docker/.env.example was deliberately not touched — it matches this repo's no-env-file pre-commit gate ((^|/)\.env([._-].+)?$), which rejects any staged .env* path; the required-var contract lives in the compose file, the CI-gate-linked test fixture, and the two safe (.txt/.md) doc surfaces instead.

Scope note

Static change + tests only, per ticket. No .201 lane was brought up, restarted, or probed; no live runtime mutation of any kind.

Verification

  • RED proof: reverted the compose fix locally, reran test_dev_redpanda_advertise_host_fails_fast_when_unset → failed on the old ${DEV_REDPANDA_ADVERTISE_HOST:-localhost} string. Restored the fix → passed.
  • uv run pytest tests/unit/infra/ -n auto (162 tests) — all green.
  • uv run pytest tests/integration/docker/ tests/integration/infra/ — 9 passed, 37 skipped (docker unavailable on this runner; same skip pattern as sibling prod/stability/judge render tests, will execute in CI).
  • uv run ruff format --check / uv run ruff check (full src/ + tests/) — clean.
  • uv run mypy src/omnibase_infra/ — clean (no src/ files touched by this change).
  • uv run pre-commit run --all-files — clean except three pre-existing, unrelated findings verified independent of this diff (none of the flagged files are touched here): a URL-authority-gate ratchet drift on 3 files with pre-existing localhost docstring examples, an SPDX-header year mismatch (2025 vs 2026) on 2 unrelated test files, and a .200-machine-local GITHUB_TOKEN env gap unrelated to DEV_REDPANDA_ADVERTISE_HOST (which itself is satisfied, not flagged). A second pre-commit run scoped to exactly this diff's changed files passed clean except that same pre-existing GITHUB_TOKEN gap.
  • Governed test selector (scripts/ci/detect_test_paths.py, not a hand-typed -k) selected tests/unit/infra/ for this diff — run above.

Closes OMN-15173

Evidence-Ticket: OMN-15173
Evidence-Source: OCC#5128

docker-compose.infra.yml silently defaulted DEV_REDPANDA_ADVERTISE_HOST to
localhost via ${DEV_REDPANDA_ADVERTISE_HOST:-localhost} whenever the env var
was unset. The dev lane runs on .201 as a shared host (deploy-runtime.sh runs
it as infra.yml alone, with no overlay), so an unset var advertised a broker
address unreachable by any off-host client (CI runner, another machine) while
looking healthy locally on .201 itself.

Switch both advertise-kafka-addr and advertise-pandaproxy-addr to the compose
:? fail-fast form, matching the existing PROD_REDPANDA_ADVERTISE_HOST
precedent in docker-compose.prod.yml and the check_required_env_vars.py /
check-required-env-vars pre-commit gate already enforced on this file. Add
DEV_REDPANDA_ADVERTISE_HOST to the test_compose_config_valid fixture (kept in
sync by test_all_required_compose_vars_in_fixture) and to docker/README.md +
docker/env-example-full.txt as a required var.

Static regression test proves both states: RED against the pre-fix compose
file (silent ${VAR:-localhost} present), GREEN after the fix (:? required
form present, old default gone). New non-mutating docker-compose-config-render
integration tests (test_dev_runtime_compose_render.py, skipped where docker
is unavailable, matching the sibling prod/stability/judge render tests)
additionally prove: an unset var fails the render, and an explicit value is
honored verbatim (never silently overridden with localhost).

Closes OMN-15173
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5adf6f57-35d4-4b45-9b6f-f84feea20479

📥 Commits

Reviewing files that changed from the base of the PR and between f7fb7cd and 0d92c1f.

📒 Files selected for processing (6)
  • docker/README.md
  • docker/docker-compose.infra.yml
  • docker/env-example-full.txt
  • tests/integration/docker/test_docker_integration.py
  • tests/integration/infra/test_dev_runtime_compose_render.py
  • tests/unit/infra/test_runtime_lane_kafka_environment.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-15173-dev-advertise-host-failfast

Comment @coderabbitai help to get the list of available commands.

jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request Jul 27, 2026
…ibase_infra#2495 (#5110)

* evidence(OMN-15173): author OCC companion for OmniNode-ai/omnibase_infra#2495

OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 0d92c1f8203cd97469128abb4ff4789e050033c3.

* evidence(OMN-15173): self-bind OCC#5110 + rebind contract_sha256

---------

Co-authored-by: omnimarket-bot <bot@omninode.ai>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8468/OMN-8524) and do not block merge. Error: all review endpoints [192.168.86.201:8000 192.168.86.201:8001 ] unreachable — preflight short-circuit (no models available)


Gate semantics (pilot phase)

Verdict Meaning Blocks merge?
passed No critical findings No
blocked CRITICAL findings found Yes
degraded All models unavailable (infra) No (pilot)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524)

@jonahgabriel
jonahgabriel merged commit d217cdb into dev Jul 27, 2026
159 of 170 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-15173-dev-advertise-host-failfast branch July 27, 2026 14:43
jonahgabriel added a commit that referenced this pull request Jul 27, 2026
…ender fixture + generalize the required-env coverage gate to all four (#2504)

OMN-15173 (#2495) switched docker-compose.infra.yml to the compose `:?`
fail-fast form for DEV_REDPANDA_ADVERTISE_HOST and added the var to exactly
one render fixture. Redpanda`s `command:` block is interpolated by
`docker compose config` regardless of `--profile`, so every layered render
started exiting non-zero on hosted CI: 12 failures in Tests (Split 2/15),
cascading to CI Tests Gate -> Test-Failure Ratchet Gate -> the required
CI Summary, on any PR whose selector escalated to the full suite.

Fix (1): supply the var (render-only `localhost`) in the three hermetic
render fixtures - prod, judge, stability-test. The compose fail-fast is
UNCHANGED; nothing regains a silent default.

Fix (2), the actual defect: tests/ci/test_compose_required_env_coverage.py
exists to catch "a `:?` var was added to compose but not to the fixture"
(OMN-5240) and did not fire, because FIXTURE_FILE was hardcoded to one of
four render fixtures. It now checks every registered fixture (env dict keys
union `--env-file` contents), fails closed on any unregistered
`tests/integration/**/*compose_render*.py` module, and carries an explicit,
justified `intentionally_unset` hatch for the dev fixture that must NOT set
the var (its OMN-15173 counter-test proves the unset render fails).

Also adds test_dev_advertise_host_keeps_fail_fast_form: giving compose a
`:-localhost` default back would turn every render green while restoring the
off-host regression OMN-15173 removed. That shortcut is now RED, on hosts
with or without Docker.

tests/integration/docker/test_docker_integration.py: the render env dict is
lifted from the test body to module-level COMPOSE_CONFIG_RENDER_ENV so the
gate extracts it the same way as the other three. Pure move, same values.

Evidence: RED (fix 1 reverted) = 3 parametrized gate failures naming
DEV_REDPANDA_ADVERTISE_HOST and each fixture; GREEN after = 8 passed. Real
`docker compose config` renders on a Docker host: prod/judge/stability all
exit 1 without the var and exit 0 with it; dev lane still exits 1 when the
var is unset and honors an explicit value. Old gate re-run against pristine
dev: 43 required / 0 missing - it passed while CI was red.

Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
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