Repository navigation
fix(hooks): fix PostToolUse JSON validation failures and worktree path parsing (OMN-8326) - #1201
Merged
Merged
Conversation
…orktree path parsing
Three hooks were causing "JSON validation failed" errors in PostToolUse:
1. post_tool_use_commit_verify.sh — outputting {"decision":"allow","message":"..."}
(PreToolUse format) instead of passing through the tool input JSON. Fixed to
pass through $INPUT and inject reminder via hookSpecificOutput.
2. post_tool_use_state_verify.sh — outputting a plain string warning instead of
JSON, and not passing through $INPUT on non-matching paths. Fixed all exit paths
to printf '%s\n' "$INPUT" and warn via hookSpecificOutput when port check fails.
3. pre_tool_use_bash_guard.sh + bash_guard.py — worktree path extraction broke on
multi-line commands (backslash+newline continuations). The `\` character was
extracted as the worktree path token, blocking valid paths. Fixed both the shell
tokenizer (CMD_FLAT with tr -d '\\') and the Python tokenizer
(command.replace("\\\n", " ").replace("\\", "")) to strip continuations first.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 24 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes enhance bash guard hooks to properly handle multi-line git worktree commands with backslash-newline continuations, replace Python-based JSON parsing with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
jonahgabriel
enabled auto-merge
April 10, 2026 17:14
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/onex/hooks/scripts/post_tool_use_commit_verify.sh`:
- Around line 39-41: The current jq expression replaces the entire
hookSpecificOutput object, discarding any prior fields; update the echo pipeline
to merge the new message into the existing hookSpecificOutput (or create it if
missing) instead of rebuilding it—e.g. use jq to set .hookSpecificOutput to
(.hookSpecificOutput // {}) + {message: $msg} so existing fields like
additionalContext are preserved while injecting the message for PostToolUse
hooks.
In `@plugins/onex/hooks/scripts/post_tool_use_state_verify.sh`:
- Around line 72-74: The current jq expression replaces any existing
hookSpecificOutput rather than merging the warning into it; update the jq
invocation that reads from $INPUT (using INPUT and MATCHED_PORT) so it preserves
existing .hookSpecificOutput and adds/overwrites only the message field — e.g.,
set .hookSpecificOutput = (.hookSpecificOutput // {}) + {message: ("WARNING:
Command claimed success but port " + $port + " is not listening. Verify
state.")} so other reminders/context remain intact.
In `@plugins/onex/hooks/scripts/pre_tool_use_bash_guard.sh`:
- Around line 86-89: The CMD_FLAT assignment hardcodes "python3" and should use
the sourced PYTHON_CMD variable instead: in the CMD_FLAT pipeline where the
inline Python is invoked (the python3 -c "import sys; ..."), replace that
invocation with "$PYTHON_CMD" (quoted) so the script uses the canonical
interpreter exported by common.sh; keep the same -c argument, stderr redirection
(2>/dev/null) and the fallback to tr unchanged to preserve behavior if
PYTHON_CMD is unset or fails.
🪄 Autofix (Beta)
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: a1fe46c9-c82e-41df-88da-91ce97d3c928
📒 Files selected for processing (4)
plugins/onex/hooks/lib/bash_guard.pyplugins/onex/hooks/scripts/post_tool_use_commit_verify.shplugins/onex/hooks/scripts/post_tool_use_state_verify.shplugins/onex/hooks/scripts/pre_tool_use_bash_guard.sh
jonahgabriel
changed the title
jonahgabriel
changed the title
… PYTHON_CMD (OMN-8326) - post_tool_use_commit_verify.sh: merge into existing hookSpecificOutput instead of overwriting - post_tool_use_state_verify.sh: same — preserve prior hookSpecificOutput - pre_tool_use_bash_guard.sh: use $PYTHON_CMD instead of hardcoded python3
jonahgabriel
added this pull request to the merge queue
Apr 10, 2026
andywu42
pushed a commit
to andywu42/omniclaude
that referenced
this pull request
Jun 16, 2026
…y consistency gate (OmniNode-ai#1751) ## Summary Phase 0c (omniclaude side) of the skill-output-suppression slice (epic OMN-13089), ticket **OMN-13092**. Pairs with merged omnimarket PR OmniNode-ai#1201 (daemon-side topic registration). Registers the two durable-capture event types end-to-end on the hook side and adds the blocking cross-registry consistency gate: - `artifact.captured` → `onex.evt.omnimarket.artifact-captured.v1` (duty_critical) - `tool.output.captured` → `onex.evt.omnimarket.tool-output-captured.v1` (duty_critical) ### Changes | Surface | Change | |---|---| | `plugins/onex/hooks/lib/emit_client_wrapper.py` | `SUPPORTED_EVENT_TYPES` += both event types (closes F10: registered nowhere) | | `src/omniclaude/hooks/topics.py` | `TopicBase.ARTIFACT_CAPTURED` / `TOOL_OUTPUT_CAPTURED` | | `src/omniclaude/hooks/event_registry.py` | `EventRegistration` entries (partition `correlation_id`, required fields mirror the daemon registry) | | `plugins/onex/lib/event_registry/omniclaude.yaml` | Live `--event-registry` launch registry registers both events (the daemon rejects unregistered event types with "Unknown event type") | | `src/omniclaude/hooks/topic_allowlist.yaml` | Governance entries — producer-side contract ownership is omnimarket `node_emit_daemon` (contract.yaml publish_topics + registries/topics.yaml) | | `tests/hooks/test_registry_consistency.py` | **NEW blocking gate**: hook frozenset ⊆ omniclaude `EVENT_REGISTRY` ⊆ omnimarket daemon YAML; per-event fan-out topic subset; capture topics exactly duty_critical | | `.github/workflows/ci.yml` | Dedicated `registry-consistency` job: checks out `omnimarket@dev` and runs the gate against the live routing authority (wired into the Tests Gate aggregator) | ### Pre-existing gate violations The pre-existing pre-commit violations originally fixed in this branch (demo skill hardcoded topics, legacy CLAUDE_SESSION_ID read, stale SPDX years) landed on dev independently while this PR was in flight; this branch rebased onto them and now carries only the OMN-13092 registration + gate changes. ## dod_evidence - `uv run pytest tests/hooks/test_registry_consistency.py -v` → **4 passed** (frozenset⊆EVENT_REGISTRY, frozenset⊆daemon YAML, duty_critical assertion, per-event topic subset) - Full suite `uv run pytest tests/ -m "not integration" -q` → **11,117 passed, 0 failed** (integration tests require the local Redpanda bus, unavailable on this machine by design; CI runs them) - `uv run mypy src/ --config-file mypy.ini` → Success, no issues in 711 files - `uv run ruff format` / `ruff check` → clean - `pre-commit run --all-files` → exit 0 Evidence-Ticket: OMN-13092 Evidence-Source: OCC#2588 Paired OCC receipt PRs: onex_change_control OmniNode-ai#2575 (contract + registration receipts, merged), OmniNode-ai#2581 (deploy verification receipt, merged), OmniNode-ai#2589 (omniclaude PR receipt, open), OmniNode-ai#2588 (omniclaude PR binding receipt, merged) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for two new event types: artifact capture and tool output capture events, enabling enhanced durability and reliability tracking for critical system operations. * **Tests** * Implemented comprehensive cross-registry consistency validation to ensure event definitions remain synchronized between local and daemon components, preventing registration mismatches and improving overall system reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.com>
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.
Three hooks were causing PostToolUse JSON validation failures, plus a false-positive block on multi-line worktree commands.
post_tool_use_commit_verify.sh: Was outputting decision/allow format (PreToolUse format) instead of passing through tool input JSON. Fixed to pass through input and inject reminder via hookSpecificOutput.
post_tool_use_state_verify.sh: Was outputting a plain string warning (not JSON) and dropping input on all non-matching paths. Fixed all exit paths to pass through input and moved warning into hookSpecificOutput.
bash_guard.py plus pre_tool_use_bash_guard.sh: Worktree path extraction broke on multi-line commands with backslash+newline continuations. The bare backslash was extracted as the path token, blocking all multi-line worktree add commands. Fixed both tokenizers to strip the continuation sequence before splitting.
Summary by CodeRabbit
Bug Fixes
Chores