Skip to content

fix(OMN-15959): delete zombie handler_llm_cli_subprocess.py from node_llm_inference_effect - #2728

Merged
jonahgabriel merged 11 commits into
devfrom
jonah/omn-15959-delete-zombie-handler-llm-cli-subprocess
Aug 22, 2026
Merged

jonahgabriel merged 11 commits into
devfrom
jonah/omn-15959-delete-zombie-handler-llm-cli-subprocess

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Aug 12, 2026 •

Copy link
Copy Markdown
Collaborator

OMN-15959 — Delete zombie handler_llm_cli_subprocess.py

OMN-13250 (Phase D, Done 2026-07-15) claimed node_llm_inference_effect had been
stripped to HTTP-only and handler_cross_cli_invoker.py deleted. Neither claim
was true.
Live state at omnibase_infra@52d8b601c (2026-08-12): the file was
still present (12,022 bytes, mtime 2026-07-26), still exposed
inference.gemini_cli/inference.claude_cli/inference.opencode_cli, still
carried 40 passing tests. OMN-13215 had already killed the architecture this
handler served — omnimarket/src/omnimarket/configs/routing_tiers.yaml:27:
"shelled codex-cli/cli_agents tiers were REMOVED" — the handler was the
stump. Evidence-Source: docs/plans/2026-08-12-delegation-exec-backends-revival-brief.md §1.2 (omni_home).

What changed

  • Deleted src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py (316 lines) and its 3 tests files (40 tests: test_handler_llm_cli_subprocess.py 4, _claude_opencode.py 8, _execution.py 28).
  • Removed the 3 CLI operations (inference.gemini_cli/claude_cli/opencode_cli) from node_llm_inference_effect/contract.yaml handler_routing.handlers; bumped contract_version 1.4.4 → 1.5.0 (operation-surface removal); updated: 2026-08-12.
  • Removed the now-dead validation_exemptions.yaml entry keyed to the deleted file.
  • Fixed a stale doc-drift finding in .github/workflows/hostile-reviewer.yml (a required CI gate, "Hostile Review Gate"): its header comment and PR-comment template claimed the adversarial review path runs "via HandlerLlmCliSubprocess". Verified against omniintelligence/src/omniintelligence/review_pairing/cli_review.py live: zero references to handler_llm_cli_subprocess/HandlerLlmCliSubprocess anywhere in omniintelligence — the module calls model endpoints directly and only shells out to gh for PR-diff fetching. The claim was never true; corrected in place rather than left dangling post-deletion.
  • Updated docstrings in handler_coding_agent_invoke.py, handler_wiring.py (2 spots), and enum_cli_backend_status.py that named the module — either marking it explicitly deleted or genericizing the illustrative example.
  • Repointed 2 dead-file references (test_validate_pr_contract_sync.py fixture path, 2 compliance test methods removed from test_handler_autowiring_compliance.py/test_handler_autowiring_compliance_followup.py) and genericized 2 test fixtures in test_wiring.py that used the deleted handler's class name / exact operation strings as illustrative examples for the OMN-9461 "shared handler, multiple operations" ID-collision regression guard — the guarded behavior (unrelated to this handler) still has full coverage under synthetic names.
  • Left untouched, deliberately: contracts/OMN-10137.yaml (2 hits) — a per-repo deploy-evidence historical receipt tied to the original OMN-10137 PR that registered these ops; historical record-keeping, not a live reference (same exemption class as OCC ticket-ref retention).

Net: -1396/+53 lines, zero new surface.

Seams

What consumed the 3 deleted operations? Nothing — proven, not assumed.

  • omnimarket/src/omnimarket/configs/routing_tiers.yaml:27 — the delegation-tier config that would have selected these ops (cli_agents tier) was already removed under OMN-13215, before this PR. Reconfirmed live on omnimarket@main at PR time.
  • Repo-wide positive-controlled grep (post-deletion, this worktree) for the handler symbol (handler_llm_cli_subprocess/HandlerLlmCliSubprocess) and the 3 op names (inference.claude_cli/inference.gemini_cli/inference.opencode_cli) returns zero hits outside: (a) this PR's own "was deleted" annotations, and (b) the deliberately-preserved contracts/OMN-10137.yaml historical receipt.
  • pyproject.toml entry points and docker/catalog/ service manifests: zero references to the module or class (checked live, pre-deletion — nothing to unregister).
  • RegistryInfraLlmInferenceEffect.get_supported_operations() already only ever declared ["inference.openai_compatible"] — never registered the CLI ops, so no DI-registration code needed touching.

OMN-14355 canon-shape ratchet — verified live, question is moot

Ran omnibase_core/scripts/ci/canonical_handler_shape.py --package omnibase_infra --src-root <this worktree>/src --json (full classification, 116 nodes) against the post-deletion tree:

  • Zero occurrences of llm_cli_subprocess anywhere in the classification output.
  • node_llm_inference_effect classifies is_canonical: true, handler_module: ...handler_llm_inference_command (unchanged from pre-deletion — classify_node() only ever resolved handler_routing.handlers[0], so it was structurally blind to the CLI handler before deletion too — confirmed by grep on canonical_handler_shape_baseline.py, zero hits for this handler in the committed core baseline).
  • The deletion makes the question moot: there is no longer a file for the ratchet to classify or misclassify.

agent_invocation: true count

grep -rn "agent_invocation" src/omnibase_infra/nodes/*/contract.yaml post-deletion: exactly one — node_coding_agent_invoke_effect/contract.yaml:72. The other 3 coding-agent-quartet nodes declare it false; node_remote_agent_invoke_effect has an unrelated operation literally named remote_agent_invocation (not the descriptor boolean).

Test evidence

  • tests/unit/nodes/node_llm_inference_effect/ (full node suite): 462 passed, 0 failed.
  • tests/unit/runtime/auto_wiring/ + tests/unit/nodes/node_coding_agent/ + tests/scripts/test_validate_pr_contract_sync.py: 554 passed, 0 failed.
  • tests/integration/{test_projection_handler_wiring_runtime_dispatch,runtime/test_handler_wiring_resolver_integration,runtime/test_handler_wiring_async_incompat_quarantine_integration,runtime/test_handler_wiring_entry_keys_integration,runtime/test_llm_inference_contract_runtime_bus,projectors/test_handler_wiring_projection_dispatch,handlers/test_handler_autowiring_compliance,test_handler_autowiring_compliance_followup}.py: 28 passed, 1 skipped (Kafka integration test, opt-in by design — KAFKA_INTEGRATION_TESTS=1).
  • tests/unit/ full tree (-n auto): 23300 passed, 40 skipped (all pre-existing environment-conditional skips — omnimarket-sibling-not-installed, Docker-daemon-required, flock-unavailable — none touch this change).
  • tests/ci/ tests/scripts/ tests/audit/ tests/services/ tests/replay/ tests/chaos/ tests/performance/: 2 pre-existing failures, both orthogonal and unrelated to this diff (verified: zero files in this PR touch either surface):
    • tests/ci/test_env_parity.py::test_k8s_bound_keys_are_bound_in_compose — omnimarket k8s-vs-compose env-key parity drift (KAFKA_CONSUMER_GROUP, PROJECTION_RUNNER_HEALTH_PORT on the projection-writer deployments).
    • tests/performance/test_llm_endpoint_slo.py::TestCoder14BSlo::* — live GPU LLM endpoint returns HTTP 404 for Qwen/Qwen2.5-Coder-14B-Instruct (endpoint-availability test, not code).
  • Full tests/integration/ under -n auto showed additional failures under host contention (Unclosed AIOKafkaProducer — Kafka-connectivity signature); none named llm_inference/cli_subprocess/coding_agent in the failing set, and the directly-relevant integration coverage (listed above) is 100% green in isolation — attributed to live-service contention on the local dev box, not this diff. CI's governed selector is the authoritative gate.
  • Pre-push governed selector (OMN-13973) escalated to the full unit suite automatically (shared-module touch: handler_wiring.py) and passed: 23300 passed, 40 skipped, ... in 658.52s.
  • uv run ruff format --check / uv run ruff check: clean.
  • pre-commit run (full hook chain, on commit): all hooks passed.

DoD

  • Ticket: OMN-15959 (parent epic OMN-13245).
  • proof_class: receipt-bound (grep evidence + CI citations above; gh pr checks will be the final live proof).

Evidence-Source: OCC#6651
Evidence-Ticket: OMN-15959

…_llm_inference_effect

OMN-13250 (Phase D, Done 2026-07-15) claimed node_llm_inference_effect had
been stripped to HTTP-only and handler_cross_cli_invoker.py deleted. Neither
was true: handler_llm_cli_subprocess.py was still live with all three
CLI-shell-out operations (inference.gemini_cli/claude_cli/opencode_cli) and
40 passing tests. OMN-13215 had already killed the architecture this handler
served (omnimarket routing_tiers.yaml:27 -- "shelled codex-cli/cli_agents
tiers were REMOVED"); the handler was the stump.

- Delete handler_llm_cli_subprocess.py, its 3 operations from
  node_llm_inference_effect/contract.yaml, and its 40 tests (3 files).
- Remove the dead validation_exemptions.yaml entry for the deleted file.
- Fix a stale hostile-reviewer.yml doc-comment that named this handler as
  the review-pairing transport; omniintelligence.review_pairing.cli_review
  never actually imported it (grep-verified zero hits in omniintelligence).
- Update docstrings in handler_coding_agent_invoke.py, handler_wiring.py,
  and enum_cli_backend_status.py that referenced the now-deleted module.
- Repoint 2 dead-file references in compliance/contract-sync tests and
  genericize 2 test fixtures that used the deleted handler's symbol/op
  names as illustrative examples (test_wiring.py), preserving coverage of
  the general "shared handler, multiple operations" wiring behavior they
  guard (OMN-9461) without naming the deleted class.

Net -1396/+53 lines, zero new surface.

The sanctioned coding-agent CLI shell-out surface is node_coding_agent_invoke_effect
(descriptor.agent_invocation: true) -- and only that node, in exactly one place
in omnibase_infra now.
@coderabbitai

coderabbitai Bot commented Aug 12, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

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

Your current included review allowance is based on your included PR review attempts over the past 7 days.

Next review available in: 38 minutes

Limit details: You’ve used the included review currently available. Your 125 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

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?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 80995376-13dd-4268-b916-a3fb537d5d1e

📥 Commits

Reviewing files that changed from the base of the PR and between 0ff5b8e and caf6280.

📒 Files selected for processing (20)
  • .github/workflows/hostile-reviewer.yml
  • docs/evidence/OMN-12548/determinism-audit.json
  • scripts/ci/infra-node-allowlist.txt
  • src/omnibase_infra/models/coding_agent/enum_cli_backend_status.py
  • src/omnibase_infra/nodes/node_coding_agent_invoke_effect/contract.yaml
  • src/omnibase_infra/nodes/node_coding_agent_invoke_effect/handlers/handler_coding_agent_invoke.py
  • src/omnibase_infra/nodes/node_llm_inference_effect/contract.yaml
  • src/omnibase_infra/nodes/node_llm_inference_effect/handlers/handler_llm_cli_subprocess.py
  • src/omnibase_infra/runtime/auto_wiring/handler_wiring.py
  • src/omnibase_infra/validation/validation_exemptions.yaml
  • tests/fixtures/dispatch_parity/baseline-selection-v2.json
  • tests/integration/handlers/test_handler_autowiring_compliance.py
  • tests/integration/test_handler_autowiring_compliance_followup.py
  • tests/integration/test_omn12816_main_promotion_coverage.py
  • tests/integration/test_omn_6659_type_suppression_contract_touch.py
  • tests/scripts/test_validate_pr_contract_sync.py
  • tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess.py
  • tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_claude_opencode.py
  • tests/unit/nodes/node_llm_inference_effect/handlers/test_handler_llm_cli_subprocess_execution.py
  • tests/unit/runtime/auto_wiring/test_wiring.py

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

@github-actions

github-actions Bot commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

❓ Hostile Reviewer — UNKNOWN

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


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 — multi-model adversarial review (OMN-8468/OMN-8524)

@onexbot-occ-writer

Copy link
Copy Markdown
Contributor

OCC autobind did not mint a companion for this PR: no changed-file candidate could be proven RED against the merge base, and emitting a PR-existence probe instead would be non-falsifiable evidence (OMN-15247). Hand-authored evidence is required.

jonahgabriel and others added 2 commits August 13, 2026 08:14
…ed handler

Handler_llm_cli_subprocess.py was deleted in this PR but its allowlist
entry in scripts/ci/infra-node-allowlist.txt was left behind, tripping
the "Reject unallowlisted infra node handlers" gate (Infra Node Handler
Ownership check) with a stale-entry error.
jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request Aug 18, 2026
#6402)

* evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2728

* evidence: OCC companion self-bind for #6402

---------

Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
Co-authored-by: Jonah Gray <jonah@omninode.ai>
…t contract

Two follow-ups to the handler_llm_cli_subprocess.py deletion:

1. baseline-selection-v2.json pinned the pre-deletion selection for the
   llm-inference-request topic, which still listed the 3 deleted CLI
   dispatchers (gemini_cli/claude_cli/opencode_cli) alongside the surviving
   openai_compatible + inference_command handlers. Regenerated via the
   documented harness (`uv run python -m tests.fixtures.dispatch_parity.harness
   --out ...`). Diff verified probe-by-probe: only the 2 llm-inference-request
   probes changed, only the 3 deleted dispatcher_ids removed from each
   selection tuple -- registered_dispatchers 114->111, registered_routes
   124->121 (exactly -3, matching the deletion), zero other probes touched.
   tests/integration/runtime/test_dispatch_selection_parity.py: 10/10 pass.

2. Wave C contract-sync gate (OMN-8915) flagged this PR's docstring update to
   handler_coding_agent_invoke.py (references the deleted handler by name)
   as a handler change with no matching contract.yaml touch. Bumped
   node_coding_agent_invoke_effect's contract_version/node_version patch
   (1.0.1 -> 1.0.2) -- docstring-only, no behavioral or schema change.

Evidence-Ticket: OMN-15959
Evidence-Source: OCC#6402
@jonahgabriel
jonahgabriel enabled auto-merge (squash) August 18, 2026 12:17
…-zombie-handler-llm-cli-subprocess

# Conflicts:
#	tests/fixtures/dispatch_parity/baseline-selection-v2.json
…-zombie-handler-llm-cli-subprocess

# Conflicts:
#	docs/evidence/OMN-12548/determinism-audit.json
#	tests/fixtures/dispatch_parity/baseline-selection-v2.json
…_effect

This PR bumps node_llm_inference_effect's contract_version 1.4.4 -> 1.5.0
and metadata.updated -> 2026-08-12 (operation-surface removal from
deleting the 3 CLI ops). Two downstream integration tests hardcode the
exact prior values as staleness guards and needed updating to match:
tests/integration/test_omn12816_main_promotion_coverage.py (contract_version
pin) and tests/integration/test_omn_6659_type_suppression_contract_touch.py
(metadata.updated pin, node_llm_inference_effect entry only -- the other
4 nodes in that dict are untouched by this PR). Both verified passing
locally after the fix.
@jonahgabriel
jonahgabriel merged commit f3c5e91 into dev Aug 22, 2026
186 of 196 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-15959-delete-zombie-handler-llm-cli-subprocess branch August 22, 2026 23:59
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