Skip to content

fix(OMN-10569): strip baked LAN/path/identity defaults from omnimarket source - #546

Merged
jonahgabriel merged 5 commits into
mainfrom
jonah/omn-10569-task-3-strip-baked-lanpathidentity-defaults-from-omnimarket
May 7, 2026
Merged

jonahgabriel merged 5 commits into
mainfrom
jonah/omn-10569-task-3-strip-baked-lanpathidentity-defaults-from-omnimarket

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented May 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes OMN-10569. Strips all hardcoded LAN IPs, user-specific paths, and model ID literals from src/omnimarket/ production source (36 files).

What was removed:

  • 192.168.86.200 / 192.168.86.201 as os.environ.get fallback defaults → replaced with ""
  • /Users/jonah/Code/omni_home and /Volumes/PRO-G40/Code/omni_home → os.environ.get("OMNI_HOME", "")
  • localhost:9092 Kafka broker defaults across all 7 consumer entrypoints → ""
  • glm-4.5 hardcoded model ID default → "" (3 locations)
  • Path.home() / "Code" / "omni_home" path construction as default → ""
  • jonah@192.168.86.201 SSH target default → os.environ.get("ONEX_INFRA_SSH_TARGET", "")

Pattern: All replaced with empty-string defaults. Callers that need a value must set the env var explicitly. Probes/consumers that previously silently used a wrong machine now either skip cleanly (probes) or fail loudly at startup (consumers).

Remaining docstring/comment references to IPs in test assertions (`test_model_policy.py`) are in `tests/` scope (task #1).

Test plan

  • `uv run ruff format src/ tests/ && uv run ruff check --fix src/ tests/` — clean
  • `OMNIBASE_INFRA_PATH=... pre-commit run --all-files` — all hooks pass
  • `uv run pytest tests/ -v -m "not kafka" --ignore=tests/test_datasource_postgres.py` — 3425 passed, 0 new failures
  • Pre-existing `test_datasource_postgres.py::TestPostgresDataSourceIntegration::test_connect_and_get_row_count` failure confirmed on main (missing DB table, unrelated to this change)

dod_evidence

ticket: OMN-10569
check_type: grep_absent
pattern: "192\.168\.(86|0)\.[0-9]+"
scope: src/omnimarket/
result: no matches in os.environ.get defaults

Evidence-Source

OMN-10569

Evidence-Source: OCC#773
Evidence-Ticket: OMN-10569

Summary by CodeRabbit

  • Configuration Changes

    • Removed built-in environment defaults throughout the platform; Kafka brokers, LLM endpoints, SSH targets, and state directories now require explicit environment variable configuration.
    • Eliminated hardcoded internal IP addresses and local filesystem fallback paths that were previously used when environment variables were unset.
  • Documentation

    • Added compliance contract documentation.

@coderabbitai

coderabbitai Bot commented May 7, 2026 •

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 04d53b47-e69e-4ca0-91ba-4e7624cf2ddf

📥 Commits

Reviewing files that changed from the base of the PR and between b4dbcbf and cd2d9bf.

📒 Files selected for processing (36)
  • docs/work-tracking/contracts/OMN-10569.yaml
  • src/omnimarket/cli/cli_ab_compare_suite.py
  • src/omnimarket/nodes/node_ab_inference_effect/models/model_inference_request.py
  • src/omnimarket/nodes/node_aislop_sweep/__main__.py
  • src/omnimarket/nodes/node_autopilot_orchestrator/consumer.py
  • src/omnimarket/nodes/node_baseline_capture/handlers/probes/probe_kafka_topics.py
  • src/omnimarket/nodes/node_baseline_capture/handlers/probes/probe_system_health.py
  • src/omnimarket/nodes/node_build_loop_orchestrator/assemble_live.py
  • src/omnimarket/nodes/node_build_loop_orchestrator/consumer.py
  • src/omnimarket/nodes/node_build_loop_orchestrator/handlers/adapter_delegation_router.py
  • src/omnimarket/nodes/node_build_loop_orchestrator/handlers/assemble_live.py
  • src/omnimarket/nodes/node_compliance_sweep/__main__.py
  • src/omnimarket/nodes/node_database_sweep/handlers/handler_database_sweep.py
  • src/omnimarket/nodes/node_dispatch_queue_drainer/handlers/handler_dispatch_queue_drainer.py
  • src/omnimarket/nodes/node_doc_freshness_sweep/handlers/handler_doc_freshness_sweep.py
  • src/omnimarket/nodes/node_dod_verify/services/evidence_collector.py
  • src/omnimarket/nodes/node_duplication_sweep/handlers/handler_duplication_sweep.py
  • src/omnimarket/nodes/node_e2e_orchestrator/consumer.py
  • src/omnimarket/nodes/node_environment_health_scanner/handlers/handler_environment_health_scanner.py
  • src/omnimarket/nodes/node_handoff_effect/handlers/handler_handoff_effect.py
  • src/omnimarket/nodes/node_merge_sweep_compute/consumer.py
  • src/omnimarket/nodes/node_nightly_loop_controller/models/model_nightly_loop.py
  • src/omnimarket/nodes/node_overnight/consumer.py
  • src/omnimarket/nodes/node_platform_diagnostics/handlers/dimension_checks.py
  • src/omnimarket/nodes/node_platform_readiness/handlers/dimension_checks.py
  • src/omnimarket/nodes/node_platform_readiness/handlers/handler_platform_readiness.py
  • src/omnimarket/nodes/node_pr_lifecycle_fix_effect/handlers/adapter_pr_polish_dispatch.py
  • src/omnimarket/nodes/node_pr_lifecycle_orchestrator/consumer.py
  • src/omnimarket/nodes/node_pr_polish/workflow_runner.py
  • src/omnimarket/nodes/node_pr_snapshot_effect/handlers/handler_pr_snapshot.py
  • src/omnimarket/nodes/node_projection_llm_cost/consumer.py
  • src/omnimarket/nodes/node_runtime_sweep/__main__.py
  • src/omnimarket/nodes/node_session_orchestrator/consumer.py
  • src/omnimarket/nodes/node_session_orchestrator/handlers/handler_session_orchestrator.py
  • src/omnimarket/nodes/node_thread_reply_effect/handlers/handler_thread_reply.py
  • src/omnimarket/projection/runner.py

📝 Walkthrough

Walkthrough

This PR systematically removes hardcoded internal IP addresses, localhost defaults, and home-directory fallbacks across 40+ files, replacing them with environment-variable-driven configuration. All defaults now resolve to empty strings when environment variables are unset, requiring explicit configuration in deployment environments. A new contract (OMN-10569) documents the expected outcome and verification checks.

Changes

Remove Hardcoded Environment Defaults

Layer / File(s) Summary
Contract & Evidence Definition
docs/work-tracking/contracts/OMN-10569.yaml
New contract document specifies DoD evidence checks: unit test pass, grep verification for 192.168.* defaults in src/omnimarket/, and runtime assertion that LLM registry URLs default to empty strings.
Documentation Examples
src/omnimarket/nodes/node_ab_inference_effect/models/model_inference_request.py, src/omnimarket/nodes/node_session_orchestrator/handlers/handler_session_orchestrator.py
Remove hardcoded internal IP/port examples from docstrings and field descriptions; replace with generic placeholders like <host>:<port>.
Path & SSH Configuration Defaults
src/omnimarket/nodes/node_aislop_sweep/__main__.py, src/omnimarket/nodes/node_build_loop_orchestrator/assemble_live.py, src/omnimarket/nodes/node_compliance_sweep/__main__.py, src/omnimarket/nodes/node_database_sweep/handlers/handler_database_sweep.py, src/omnimarket/nodes/node_dispatch_queue_drainer/handlers/handler_dispatch_queue_drainer.py, src/omnimarket/nodes/node_doc_freshness_sweep/handlers/handler_doc_freshness_sweep.py, src/omnimarket/nodes/node_dod_verify/services/evidence_collector.py, src/omnimarket/nodes/node_duplication_sweep/handlers/handler_duplication_sweep.py, src/omnimarket/nodes/node_environment_health_scanner/handlers/handler_environment_health_scanner.py, src/omnimarket/nodes/node_handoff_effect/handlers/handler_handoff_effect.py, src/omnimarket/nodes/node_platform_readiness/handlers/handler_platform_readiness.py, src/omnimarket/nodes/node_pr_lifecycle_fix_effect/handlers/adapter_pr_polish_dispatch.py, src/omnimarket/nodes/node_pr_polish/workflow_runner.py, src/omnimarket/nodes/node_pr_snapshot_effect/handlers/handler_pr_snapshot.py, src/omnimarket/nodes/node_runtime_sweep/__main__.py
Refactor OMNI_HOME, ONEX_STATE_DIR, BUILD_LOOP_OUTPUT_DIR, and SSH targets (_SSH_HOST, _INFRA_SSH_TARGET) to read from environment variables with empty-string defaults, removing home-directory derived fallbacks and hardcoded SSH hostnames.
Kafka Broker Defaults
src/omnimarket/nodes/node_autopilot_orchestrator/consumer.py, src/omnimarket/nodes/node_build_loop_orchestrator/consumer.py, src/omnimarket/nodes/node_e2e_orchestrator/consumer.py, src/omnimarket/nodes/node_merge_sweep_compute/consumer.py, src/omnimarket/nodes/node_overnight/consumer.py, src/omnimarket/nodes/node_pr_lifecycle_orchestrator/consumer.py, src/omnimarket/nodes/node_session_orchestrator/consumer.py, src/omnimarket/nodes/node_projection_llm_cost/consumer.py, src/omnimarket/projection/runner.py
Update 9 consumer files to read KAFKA_BOOTSTRAP_SERVERS with fallback to KAFKA_BROKER; remove hardcoded localhost:9092 default so broker becomes empty string when both env vars are unset.
LLM Endpoints & Model Names
src/omnimarket/cli/cli_ab_compare_suite.py, src/omnimarket/nodes/node_build_loop_orchestrator/handlers/adapter_delegation_router.py, src/omnimarket/nodes/node_build_loop_orchestrator/handlers/assemble_live.py, src/omnimarket/nodes/node_nightly_loop_controller/models/model_nightly_loop.py, src/omnimarket/nodes/node_thread_reply_effect/handlers/handler_thread_reply.py
Remove hardcoded defaults for LLM_GLM_MODEL_NAME ("glm-4.5" → ""), LLM_CODER_URL, LLM_CODER_FAST_URL, and LLM base registry URLs; all now read from environment with empty-string defaults when unset.
Health Probes & Service Configuration
src/omnimarket/nodes/node_baseline_capture/handlers/probes/probe_kafka_topics.py, src/omnimarket/nodes/node_baseline_capture/handlers/probes/probe_system_health.py, src/omnimarket/nodes/node_platform_diagnostics/handlers/dimension_checks.py, src/omnimarket/nodes/node_platform_readiness/handlers/dimension_checks.py, src/omnimarket/nodes/node_projection_llm_cost/consumer.py
Update system-health probe, Redpanda admin API configuration, platform diagnostics and readiness checks, and PostgreSQL DSN to read endpoints/hosts exclusively from environment variables; skip or return empty results when configuration is missing instead of using hardcoded fallback addresses.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly Related PRs

  • OmniNode-ai/omnimarket#547: Related PR that also addresses removal of hardcoded lab/internal defaults and adds allowlist/annotation checks against literals.

Poem

🐰 Hop hop, the defaults now are gone,
No more IPs from days of old,
Environment calls the tune,
Empty strings when not configured—
A cleaner path, more secure, bold! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-10569-task-3-strip-baked-lanpathidentity-defaults-from-omnimarket

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

@github-actions
github-actions Bot enabled auto-merge May 7, 2026 14:57
@github-actions

github-actions Bot commented May 7, 2026 •

Copy link
Copy Markdown

⚠️ Hostile Reviewer — DEGRADED (informational)

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

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8524) and do not block merge. Error: cli_review exited non-zero (all models failed or binary error)


Gate semantics (pilot phase)

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

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

…t source

Replace all hardcoded 192.168.x.x IPs, /Users/jonah, /Volumes/PRO-G40 paths,
localhost:9092 broker defaults, and glm-4.5 model ID literals with empty-string
defaults. Affected surfaces: probe_system_health, probe_kafka_topics, all
consumer entrypoints, platform readiness/diagnostics dimension_checks,
node_nightly_loop_controller, node_handoff_effect, and 8 sweep node __main__.py
entrypoints. Pre-existing postgres integration test failure confirmed on main.
…efaults-strip PR

Adds docs/work-tracking/contracts/OMN-10569.yaml with dod-deploy item
(docker exec import check) so the deploy-gate can verify the env-var
default cleanup does not break runtime handler imports.
…ontract summary

test_work_tracking_contracts_have_no_user_paths bans /Users/<name> and
/Volumes/<name> in docs/work-tracking/contracts/. The summary field used
descriptive text containing those patterns; replace with generic terms.
@jonahgabriel
jonahgabriel force-pushed the jonah/omn-10569-task-3-strip-baked-lanpathidentity-defaults-from-omnimarket branch from 75f4fe3 to cd2d9bf Compare May 7, 2026 19:21
@jonahgabriel
jonahgabriel added this pull request to the merge queue May 7, 2026
Merged via the queue into main with commit 88c94b9 May 7, 2026
28 of 29 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-10569-task-3-strip-baked-lanpathidentity-defaults-from-omnimarket branch May 7, 2026 19:27
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