[NA] [SDK] fix: entry point resolves to module so pip-install works (0.1.1) + pip E2E coverage - #24
Merged
Merged
Conversation
…upport) + 0.1.1
The hermes_agent.plugins entry point was 'opik = opik_hermes:register', which
resolves to the register FUNCTION. Hermes loads a plugin via ep.load() then
getattr(<result>, 'register') expecting a MODULE — so it found no register(),
wired 0 hooks, and a pip-installed plugin produced no traces (the directory-
install path was unaffected, which is why it slipped through). Fix: point the
entry point at the module ('opik = opik_hermes'); getattr(module, 'register')
then resolves. Verified end-to-end: real Hermes + pip-installed wheel, enabled
via config.yaml plugins.enabled:[opik], logs a fresh staging trace (7 spans).
- pyproject: entry point -> opik_hermes; version -> 0.1.1
- publish.yml: assert the built wheel's entry point == 'opik_hermes' (guards the regression)
- e2e: new pip/wheel path (Dockerfile.wheel + run_e2e_wheel.sh + CI job) that
builds the wheel, pip-installs it, and drives a turn via ENTRY-POINT discovery
(no plugin dir copied) — the coverage that would have caught this. The existing
mock E2E keeps testing the directory-install path; both are supported.
- README (root + plugin): pip path enables via ~/.hermes/config.yaml
plugins.enabled:[opik] (entry-point name 'opik'), not 'hermes plugins enable'
(which only sees directory plugins); dir/GitHub install noted as separate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
The build job's wheel check still asserted 'opik_hermes:register'; after the entry-point fix the value is 'opik_hermes'. Update it (mirroring publish.yml) to assert the entry point resolves to the module. This is the same guard the fix adds — CI correctly failed on the stale string. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
baz-reviewer
Bot
dismissed
their stale review
July 8, 2026 00:31
Baz dismissed its prior approval because a re-review found new findings.
…view) Baz flagged that the 'entry point must resolve to opik_hermes (module)' guard was duplicated in ci.yml and publish.yml — an entry-point change would need updating in both. Extract it into e2e/assert_entrypoint.py (single source of truth) and call it from both workflows. Left e2e/Dockerfile.wheel's guard as-is: it verifies the INSTALLED plugin loads in-container (a different check), and copying a script into that build context isn't worth the coupling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous edit replaced the heredoc body with a single command but left the trailing 'PY' line, which YAML folded into the run: command (python e2e/assert_entrypoint.py PY -> 'no wheel matched PY'). Remove it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… (address review) Baz flagged that the entry-point check existed in two places — assert_entrypoint.py (static wheel-metadata parse) and Dockerfile.wheel (behavioral ep.load()). Valid: the behavioral check is strictly stronger (it verifies the plugin actually LOADS as a module with a callable register, exactly as Hermes does), and it subsumes the metadata check. Unify on the strong one: - Rewrite assert_entrypoint.py to do the behavioral check against the INSTALLED package: entry_points()[opik].load() must be a module with callable register. - ci.yml + publish.yml: install the built wheel, then run the shared check (previously they only string-parsed the wheel — now they verify real loadability). - Dockerfile.wheel: COPY and run the same shared script instead of an inline duplicate; run_e2e_wheel.sh stages it into the build context. One correct way to assert the contract, used everywhere, and stronger than before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
User description
Details
The bug:
pip install opik-hermesproduced no traces in Hermes. Root cause: thehermes_agent.pluginsentry point wasopik = opik_hermes:register, which resolves to theregisterfunction. Hermes loads a plugin viaep.load()thengetattr(<result>, "register")— expecting a module. So it found noregister(), wired 0 hooks, and the plugin silently did nothing. (The directory-install path imports__init__.pyas a module and was unaffected — which is exactly why this slipped through CI.)The fix (one line): point the entry point at the module —
opik = opik_hermes.getattr(module, "register")then resolves (the package exportsregister). Verified end-to-end: real Hermes + pip-installed wheel, enabled viaconfig.yamlplugins.enabled: [opik]→ fresh staging trace, 7 spans (llm=4, tool=3, NA=0),OPIK: Started logging traces....This makes both install paths work — pip/entry-point (fixed here) and directory (
hermes plugins install …, already worked).Change checklist
Issues
NA — found during dogfood/release verification. Related: OPIK-7254 (dir/GitHub install path).
Testing
pyproject: entry point →opik_hermes; version → 0.1.1.e2e/assert_entrypoint.py: reproduces Hermes' own loader —entry_points()["opik"].load()must return a module with a callableregister. Used byci.yml,publish.yml, and the E2E image, so a regression back to:registerfails the build. (Consolidated from earlier per-file checks per review.)ci.yml+publish.yml: build the wheel, install it, then run the shared behavioral check (previously only string-parsed the wheel metadata — now verifies real loadability).e2e/Dockerfile.wheel+e2e/run_e2e_wheel.sh+ a CI job ine2e.yml): builds the wheel from source,pip installs it into Hermes, enables viaconfig.yaml, drives a turn with entry-point discovery only (no plugin dir copied), asserts the journal. This is the coverage that would have caught the bug. The existing mock E2E keeps testing the directory-install path — CI now covers both.E2E via pip (entry point),E2E against latest Hermes,E2E against real Opik, tests (3.11/3.12/3.13), lint; Baz re-reviewed clean.Documentation
~/.hermes/config.yamlplugins.enabled: [opik](entry-point nameopik), nothermes plugins enable(that CLI only sees directory-installed plugins). The directory/GitHub install is noted as a separate path.🤖 Generated with Claude Code
Generated description
Fixes the Hermes plugin entry point so
opikmaps to theopik_hermesmodule, allowing the loader’sgetattr(<module>, "register")to succeed for both pip and directory installs while bumping the package to 0.1.1. Guards this contract through the sharedassert_entrypoint.pycheck, plus new pip-entry-point E2E coverage and doc updates for the pip install path.run_e2e_wheel.sh, and wire CI/workflow steps to run the new scenario and reuse the entry-point assertion.Modified files (6)
Latest Contributors(1)
opikto theopik_hermesmodule, documenting the pip workflow, and adding the sharedassert_entrypoint.pyguard so Hermes can alwaysgetattr(module, "register")when loading the plugin.Modified files (4)
Latest Contributors(1)