Repository navigation
feat(OMN-9143): grep guard + CI gate blocking polymorphic-agent in enforcement paths - #1795
Conversation
…forcement paths
The legacy 'polymorphic-agent' dispatch target was already scrubbed from the
omniclaude hook enforcement surface (now 'onex:'-prefixed subagents). This lands
the regression guard half of OMN-9143 (Pattern G) so it cannot reappear:
- plugins/onex/hooks/scripts/grep_guard_no_polymorphic_agent.sh: fail-closed
grep guard over plugins/onex/hooks/{scripts,lib} + hooks.json. Full-tree mode
and pre-commit file-mode (scoped to the enforcement surface).
- tests/unit/hooks/test_no_polymorphic_agent_references.py: 0-offender assertion
+ guard agreement + injected-offender negative test.
- .pre-commit-config.yaml: no-polymorphic-agent hook (blocking).
- .github/workflows/ci.yml: no-polymorphic-agent job wired into Quality Gate
needs + aggregation loop.
Scope: enforcement paths only. Historical observability SQL
(scripts/observability/routing_metrics.sql) and routing-fallback tests
(tests/lib/core/*) legitimately reference the retired name and are out of scope.
Docs untouched per ticket. pre_tool_use_poly_enforcer.sh not renamed (already
reference-free; rename is cosmetic and would churn deployed hooks.json wiring).
📝 WalkthroughWalkthroughAdds OMN-9143 enforcement: a new Bash grep guard script blocks reintroduction of the retired Changespolymorphic-agent Regression Guard
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
| 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)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/hooks/test_no_polymorphic_agent_references.py (1)
103-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider removing unused
tmp_pathfixture parameter.The test declares
tmp_path: Pathbut intentionally creates the offender file directly in the enforcement directory (ENFORCEMENT_ROOTS[0]) rather than using the temp directory. While the try/finally cleanup is correct, the unused fixture parameter may confuse readers.♻️ Proposed cleanup
-def test_grep_guard_detects_injected_offender(tmp_path: Path) -> None: +def test_grep_guard_detects_injected_offender() -> None: """The grep guard must fail closed on an injected enforcement-path offender."""🤖 Prompt for 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. In `@tests/unit/hooks/test_no_polymorphic_agent_references.py` around lines 103 - 120, Remove the unused `tmp_path: Path` parameter from the function signature of `test_grep_guard_detects_injected_offender`. This parameter is declared but never used in the test body since the test intentionally creates the offender file directly in `ENFORCEMENT_ROOTS[0]` rather than using a temporary directory. Simply remove it from the function definition to avoid confusing readers about the test's actual behavior.
🤖 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.
Nitpick comments:
In `@tests/unit/hooks/test_no_polymorphic_agent_references.py`:
- Around line 103-120: Remove the unused `tmp_path: Path` parameter from the
function signature of `test_grep_guard_detects_injected_offender`. This
parameter is declared but never used in the test body since the test
intentionally creates the offender file directly in `ENFORCEMENT_ROOTS[0]`
rather than using a temporary directory. Simply remove it from the function
definition to avoid confusing readers about the test's actual behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d414c49a-7185-4b6c-b28e-92410282ae36
📒 Files selected for processing (4)
.github/workflows/ci.yml.pre-commit-config.yamlplugins/onex/hooks/scripts/grep_guard_no_polymorphic_agent.shtests/unit/hooks/test_no_polymorphic_agent_references.py
Evidence-Source: 02fde8dcb5426fc6ab0083d0a0c48f581be01080
Evidence-Ticket: OMN-9143
OMN-9143 — [Pattern G] Scrub polymorphic-agent from omniclaude enforcement paths
Closes OMN-9143.
Context / verified state
The
polymorphic-agentdispatch target was already scrubbed from the omniclaude hook enforcement surface before this PR. Verified againstorigin/dev:The enforcement hooks now enforce
onex:-prefixed subagents (withgeneral-purposeallowed unprefixed) — e.g.plugins/onex/hooks/scripts/pre_tool_use_poly_enforcer.sh. What was missing — and what this PR lands — is the regression guard half of the ticket (the enforcement-not-detection requirement, CLAUDE.md Rule #5).What this PR adds
plugins/onex/hooks/scripts/grep_guard_no_polymorphic_agent.sh— fail-closed grep guard over the enforcement surface (plugins/onex/hooks/{scripts,lib}+hooks.json). Full-tree mode (CI) and pre-commit file-mode (scoped to the enforcement surface). Self-allowlisted so its own header doesn't trip it.tests/unit/hooks/test_no_polymorphic_agent_references.py— asserts 0 offenders in enforcement paths, asserts the guard agrees (exit 0), and a negative test asserting the guard fails closed (exit 1) on an injected enforcement-path offender..pre-commit-config.yaml—no-polymorphic-agenthook (blocking), scoped to enforcement-path files..github/workflows/ci.yml—no-polymorphic-agentjob wired into thequality-gateaggregator (needs+ aggregation loop). The job is a pure bash grep guard (no uv/python).Scope decisions (documented per CLAUDE.md Rule #2)
scripts/observability/routing_metrics.sql(historical transformation-rate metrics over past event rows) andtests/lib/core/*routing-fallback tests (assert the documented low-confidence fallback). These legitimately reference the retired name and are not enforcement code paths. The ticket scopes to "enforcement-critical code paths."pre_tool_use_poly_enforcer.shnot renamed. The file is already reference-free; the rename is cosmetic and would churn the deployedhooks.jsonwiring for no enforcement benefit. The ticket's primary intent — nopolymorphic-agentin enforcement paths + a regression guard — is fully achieved.docs/architecture/*/ ADR / getting-started / reference), per the ticket DoD.OMNICLAUDE_HOOKS_DISABLEkill-switch (OMN-9140) preserved — not touched.no-polymorphic-agenttomainprotection): noted as a repo-settings action; the CI job is wired into the API-stableQuality Gateaggregator, which is already a required check, so the guard blocks merge today via that aggregator.dod_evidence
Notes
tests/unit/hooks/test_mode_resolution.py::test_auto_detect_external_projectfails locally inside the worktree (mode resolver returnsfullbecause the worktree path is classified as an omninode repo; the test hardcodeslitefor an external-project layout). It is byte-identical toorigin/dev(git diff origin/dev -- tests/unit/hooks/test_mode_resolution.pyis empty) and unrelated to this changeset; it passes in CI's clean external-project runner.Summary by CodeRabbit
Tests
Chores
Receipt Gate / OCC pairing
Paired OCC evidence PR: OmniNode-ai/onex_change_control#2962 (contract
contracts/OMN-9143.yaml+ DoD receipts binding to this PR head, commitea01ddc36). MERGED toonex_change_control@mainas merge-commit02fde8dcb5426fc6ab0083d0a0c48f581be01080.Evidence-Sourceabove is repointed to that OCC merge-commit SHA (per the OCC-squash-invalidates-Evidence-Source rule — the pre-squash branch SHA is not used). Theverify / Run Receipt-Gate"Resolve Evidence-Source" step now resolves against the durable OCC merge commit that containscontracts/OMN-9143.yamland the OMN-9143 DoD receipts.