Repository navigation
fix(OMN-15960): delete duplicate node_claude_code_session_effect - #1985
Merged
jonahgabriel merged 3 commits intoAug 12, 2026
Merged
Conversation
Adjudicates node_claude_code_session_effect (omniclaude) against
node_coding_agent_invoke_effect (omnibase_infra quartet) per the
2026-08-12 delegation-exec-backends revival brief §1.3.
Finding: the node is not a bus-orphan like the quartet was — its
backend_subprocess.py IS imported and called by omniclaude's own
wire_skill_dispatchers() multiplexer (109/110 skill-orchestrator
contracts declare execution.backend: claude_code), registered via
the "claude" onex.domain_plugins entry point. But that whole hosting
runtime service is never deployed anywhere: omnibase_infra's
Dockerfile.runtime does not install omniclaude; omniclaude's own
deployment/Dockerfile is explicitly "Minimal Dockerfile for CI
validation... Full deployment Dockerfile pending codebase
restructure"; no docker-compose/catalog service runs
`python -m omniclaude.runtime start`. This predates and is
superseded by the current "omniclaude = client config only" /
"skills are thin shims, logic in omnimarket nodes" doctrine.
Net: zero deployed consumers. The omnibase_infra quartet is the
canonical, image-baked coding-agent surface and can trivially cover
persistent-session semantics later if a real need appears (fold-in
child ticket filed under OMN-13245 = OMN-15962, not built here).
Deletes the node (contract, backends, models, protocols, tests,
pyproject.toml entry-point) and repairs every in-repo reference so
the tree has zero dangling imports:
- wiring_dispatchers.py: claude_code stays a valid, duck-typed,
caller-injectable backend slot (kwargs, not the deleted request
model); removes the container auto-discovery block that only
ever resolved the deleted concrete class.
- runtime/__main__.py dry-run backend check, consumer_group_guard.py
registry, canonical_handler_shape_baseline.py ratchet (legitimate
shrink), node_local_coding_orchestrator's downstream_effects list,
model_skill_result.py call-site docstring.
- test suites updated to match (82 + 45 targeted tests green).
Not touched (deliberate): the 109 node_skill_*_orchestrator
contract.yaml files (execution.backend: claude_code remains a valid
Literal value describing desired-but-unimplemented backend — same
practical state as before this PR); docs/investigations/
non-node-dirs-nodes.md (archival snapshot dated 2026-03-08).
Evidence-Source: docs/plans/2026-08-12-delegation-exec-backends-revival-brief.md (omni_home)
OMN-15960
Second half of the OMN-15960 delete — the previous commit staged
only the node_claude_code_session_effect deletions (a bad multi-path
`git add -A --` pathspec silently aborted before staging the repair
edits). This commit lands the actual repair diff:
- pyproject.toml: drop the onex.nodes entry-point registration.
- wiring_dispatchers.py: claude_code stays a valid, duck-typed,
caller-injectable backend slot (plain kwargs, not the deleted
ModelClaudeCodeSessionRequest); drops the container
auto-discovery block that only ever resolved the deleted
concrete class.
- runtime/__main__.py: dry-run backend check no longer imports the
deleted module.
- consumer_group_guard.py + its test: drop the
NodeClaudeCodeSessionEffect consumer-group entry, no tombstone
per feedback_noncanonical_nodes_get_deleted.
- canonical_handler_shape_baseline.py: legitimate ratchet shrink
(node no longer exists, not a canonical-shape flip).
- node_local_coding_orchestrator: drop the downstream_effects
reference.
- model_skill_result.py, fake_task_dispatcher.py,
test_wiring_dispatchers.py: docstring/comment accuracy.
Verified on this commit (not just the prior one): ruff
format+check, mypy on the three touched runtime modules, pre-commit
--files on the full changed set, targeted pytest (127 passed:
tests/unit/runtime/ + tests/unit/test_consumer_group_guard.py), and
a live `python -m omniclaude.runtime start --dry-run` reads
"Contracts: 131/131 OK | Backends: claude_code=REMOVED (OMN-15960;
caller-injected only) vllm=OK" — confirming the 109 skill contracts
declaring execution.backend: claude_code still parse (untouched
Literal type) while the dead backend degrades cleanly instead of
import-erroring.
OMN-15960
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. |
📝 WalkthroughWalkthroughThe PR removes the Claude Code session effect node, its contract, models, protocol, subprocess backend, and registrations. Runtime wiring now accepts caller-injected Claude backends and invokes ChangesClaude Code session effect removal
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/omniclaude/runtime/wiring_dispatchers.py`:
- Around line 943-951: Update the plugin wiring path in the function that calls
wire_skill_dispatchers so it accepts and forwards a caller-provided Claude
backend via claude_code_backend, preserving Claude-backed contract availability;
alternatively, conditionally omit Claude-backed registrations when no backend is
supplied. Ensure the existing vllm_backend wiring remains unchanged.
- Around line 388-394: Update the backend detail assignment near the
dispatcher’s async session_query flow so it reflects the caller-injected backend
rather than always reporting claude_subprocess. Derive backend_detail from the
injected backend’s type or a validated backend identifier, while preserving the
existing completion-event reporting behavior for subprocess backends.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc75a724-3139-45fc-bcb7-704897d619b7
📒 Files selected for processing (23)
.secrets.baselinepyproject.tomlscripts/ci/canonical_handler_shape_baseline.pysrc/omniclaude/lib/consumer_group_guard.pysrc/omniclaude/nodes/node_claude_code_session_effect/__init__.pysrc/omniclaude/nodes/node_claude_code_session_effect/backends/__init__.pysrc/omniclaude/nodes/node_claude_code_session_effect/backends/backend_subprocess.pysrc/omniclaude/nodes/node_claude_code_session_effect/contract.yamlsrc/omniclaude/nodes/node_claude_code_session_effect/models/__init__.pysrc/omniclaude/nodes/node_claude_code_session_effect/models/model_claude_code_session_request.pysrc/omniclaude/nodes/node_claude_code_session_effect/node.pysrc/omniclaude/nodes/node_claude_code_session_effect/protocols/__init__.pysrc/omniclaude/nodes/node_claude_code_session_effect/protocols/protocol_claude_code_session.pysrc/omniclaude/nodes/node_local_coding_orchestrator/contract.yamlsrc/omniclaude/nodes/node_local_coding_orchestrator/node.pysrc/omniclaude/runtime/__main__.pysrc/omniclaude/runtime/wiring_dispatchers.pysrc/omniclaude/shared/models/model_skill_result.pytests/support/fake_task_dispatcher.pytests/unit/nodes/node_claude_code_session_effect/__init__.pytests/unit/nodes/node_claude_code_session_effect/test_backend_subprocess.pytests/unit/runtime/test_wiring_dispatchers.pytests/unit/test_consumer_group_guard.py
💤 Files with no reviewable changes (16)
- src/omniclaude/nodes/node_claude_code_session_effect/backends/init.py
- tests/unit/nodes/node_claude_code_session_effect/init.py
- scripts/ci/canonical_handler_shape_baseline.py
- src/omniclaude/nodes/node_claude_code_session_effect/protocols/init.py
- pyproject.toml
- src/omniclaude/nodes/node_claude_code_session_effect/node.py
- src/omniclaude/nodes/node_claude_code_session_effect/models/model_claude_code_session_request.py
- src/omniclaude/nodes/node_claude_code_session_effect/contract.yaml
- src/omniclaude/nodes/node_claude_code_session_effect/protocols/protocol_claude_code_session.py
- src/omniclaude/shared/models/model_skill_result.py
- src/omniclaude/nodes/node_claude_code_session_effect/backends/backend_subprocess.py
- src/omniclaude/nodes/node_claude_code_session_effect/models/init.py
- src/omniclaude/nodes/node_local_coding_orchestrator/node.py
- src/omniclaude/nodes/node_claude_code_session_effect/init.py
- tests/unit/nodes/node_claude_code_session_effect/test_backend_subprocess.py
- src/omniclaude/nodes/node_local_coding_orchestrator/contract.yaml
jonahgabriel
enabled auto-merge (squash)
August 12, 2026 20:36
jonahgabriel
pushed a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Aug 12, 2026
…6395) * evidence: OCC companion pass 1 for OmniNode-ai/omniclaude#1985 * evidence: OCC companion self-bind for #6395 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
Contributor
|
| 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)
Two real findings from CodeRabbit review of PR #1985: 1. (Minor) wiring_dispatchers.py hardcoded backend_detail="claude_subprocess" even though the slot now accepts any duck-typed caller-injected backend post-deletion — completion events would misreport the backend type. Fixed: backend_detail = type(self._claude_code_backend).__name__. 2. (Major) plugin.py's wire_dispatchers() calls wire_skill_dispatchers() with only vllm_backend — no claude_code_backend, and the removed container-auto-discovery fallback means every claude_code contract on that runtime path now always reports BACKEND_UNAVAILABLE. Correct catch: LifecycleState never carried a claude_code_backend field (it was previously resolved via the now-removed auto-discovery block, not lifecycle state). Documented the gap explicitly at the call site rather than inventing unused DI machinery for a backend with zero remaining implementations — matches the caller-injection design already used for the constructor/wire_skill_dispatchers() parameter. Verified: pytest tests/unit/runtime/ + test_consumer_group_guard.py (127 passed), mypy on the four touched runtime modules (success), ruff format+check (clean). OMN-15960
jonahgabriel
deleted the
jonah/omn-15960-adjudicate-claude-code-session-effect
branch
August 12, 2026 21:58
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adjudicates
node_claude_code_session_effect(omniclaude) against the canonicalnode_coding_agent_invoke_effectquartet (omnibase_infra) per the 2026-08-12 delegation-exec-backends revival brief §1.3, and deletes the omniclaude node.Finding (not a bare "nothing invokes it")
Grep for the node/class found real hits — this is not a bus-orphan like the omnibase_infra quartet was before its own revival work.
backend_subprocess.py::SubprocessClaudeCodeSessionBackendis genuinely imported and called by omniclaude's ownwire_skill_dispatchers()multiplexer (src/omniclaude/runtime/wiring_dispatchers.py) — 109/110node_skill_*_orchestratorcontracts declareexecution.backend: claude_code, and the dispatcher is registered via the"claude"onex.domain_pluginsentry point thatomnibase_infra's kernel plugin loader (service_kernel.py§4.6, entry-point discovery) auto-discovers.But that entire hosting runtime service (
PluginClaude,wire_skill_dispatchers,runtime/__main__.py) is never deployed anywhere:omnibase_infra/docker/Dockerfile.runtimedoes not installomniclaudeas a package (onlyomnibase_core,omnibase_compat,onex_change_control,omnimarket).deployment/Dockerfileis explicitly labeled# Minimal Dockerfile for CI validation. Full deployment Dockerfile pending codebase restructure.python -m omniclaude.runtime start.So the entry point is real Python that would work in-process if this runtime service were ever booted, but nothing boots it. This predates and is superseded by the platform's current doctrine (
feedback_mcp_server_in_omnibase_infra: "omniclaude = client config only";feedback_skills_architecture: "skills are thin shims, logic in omnimarket nodes").Net: zero deployed consumers. The omnibase_infra quartet is the canonical, image-baked (
claude/codexCLI verified present inDockerfile.runtime), currently-deployed coding-agent surface. Session semantics (persistent multi-turn) are not supported by the quartet today, but nothing real needs them right now — the only "caller" was this same never-deployed subsystem.Decision: fold-in disposition, delete now
Per the dispatch's decision rule and
feedback_noncanonical_nodes_get_deleted: omniclaude loses the node either way. Because real (if never-deployed) wiring code existed, this is the fold-in path, not a silent delete — filed OMN-15976 under epic OMN-13245 to extend the quartet with session semantics only if/when a real caller needs it (not built speculatively here).What changed
src/omniclaude/nodes/node_claude_code_session_effect/(contract, node.py, backends/, models/, protocols/) + its test dir + thepyproject.tomlonex.nodesentry-point registration.wiring_dispatchers.py:claude_codestays a valid, duck-typed, caller-injectable backend slot (plain kwargs instead of the deletedModelClaudeCodeSessionRequest); removed the container auto-discovery block that only ever resolved the deleted concrete class.runtime/__main__.pydry-run backend check,consumer_group_guard.pyregistry (+ its test),canonical_handler_shape_baseline.pyratchet (legitimate shrink — node no longer exists, not a canonical-shape flip),node_local_coding_orchestrator'sdownstream_effectslist,model_skill_result.pycall-site docstring,fake_task_dispatcher.pydocstring.node_skill_*_orchestrator/contract.yamlfiles —execution.backend: claude_coderemains a validLiteralvalue describing a desired-but-currently-unimplemented backend, same practical state as before this PR (it was never actually available in any deployed runtime either way).docs/investigations/non-node-dirs-nodes.md— archival snapshot dated 2026-03-08, not a live inventory.Seams
No cross-repo seam: the deleted node had no
subscribe_topics(already confirmed unwired to any Kafka contract), and its only real consumer (wiring_dispatchers.py) lives in this same repo and is repaired in this PR. No other OmniNode repo importsomniclaude.nodes.node_claude_code_session_effect(verified:grep -rl node_claude_code_session_effectacross omnibase_core/omnibase_infra/omnibase_spi/omnimarket/onex_change_control/omnibase_compat returns zero source hits — only two immutable historical OCC DoD-receipt files, left untouched).Verification
uv run pytest tests/unit/runtime/ tests/unit/test_consumer_group_guard.py -m unit→ 127 passed.uv run mypy src/omniclaude/runtime/wiring_dispatchers.py src/omniclaude/runtime/__main__.py src/omniclaude/lib/consumer_group_guard.py→ success, no issues.uv run ruff format+ruff check --fixon all touched files → clean.pre-commit run --files <all touched>→ all passed.python -m omniclaude.runtime start --dry-run→Contracts: 131/131 OK | Backends: claude_code=REMOVED (OMN-15960; caller-injected only) vllm=OK— confirms the 109 skill contracts still parse (untouched Literal type) while the dead backend now degrades cleanly instead of import-erroring.OMN-15960
Evidence-Ticket: OMN-15960
Evidence-Source: OCC#6400
Summary by CodeRabbit