fix(deps): align lazy pins with audited extras + guard Discord voice (Codex PR #6, #10) - #25
Conversation
…(Codex PR #6, #10) Addresses chatgpt-codex-connector review comments on merged PRs #6 and #10. tools/lazy_deps.py (PR #6 — lazy install pins stale after Dependabot audit): - platform.slack: aiohttp 3.13.4 -> 3.14.1 (match [slack] extra) - platform.discord: add aiohttp==3.14.1 (discord.py uses aiohttp transitively; without the pin, a first-use lazy install can leave an older vulnerable aiohttp while `hermes update` considers the feature satisfied) - terminal.modal / terminal.vercel: add cbor2==6.1.2 (both backends pull cbor2 transitively; pin the audited version so existing installs refresh the vulnerable transitive package) - tool.dashboard: add starlette==1.3.1 (FastAPI pulls starlette transitively; same rationale as above) plugins/platforms/discord/adapter.py (PR #10 — voice extra no longer ships PyNaCl): - join_voice_channel now checks for PyNaCl availability before calling channel.connect(). The `voice` extra was intentionally stripped of PyNaCl (it pinned a vulnerable version and triggered Dependabot), but /voice join was still registered and would crash at runtime with an opaque missing-module error. The guard fails fast with a logged warning instructing the user to install PyNaCl>=1.6.2 manually. tests/gateway/test_discord_race_polish.py: - Added test_join_voice_returns_false_when_pynacl_missing (verifies the guard fires before channel.connect()) - Added test_join_voice_proceeds_when_pynacl_available (pins the happy path so the guard is confirmed as the regression boundary) - Updated test_concurrent_joins_do_not_double_connect to mock nacl via patch.dict (prevents sys.modules leakage into other test files) PR #3 website comments already fixed in main: serialize-javascript override is at 7.0.5, all @docusaurus/* packages aligned at 3.10.1. Verification: pytest tests/gateway/test_discord_race_polish.py tests/tools/test_lazy_deps.py tests/test_package_json_lazy_deps.py tests/tools/test_discord_tool.py (157 passed, 2 pre-existing warnings).
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
1 |
First entries
plugins/platforms/discord/adapter.py:1907: [unresolved-import] unresolved-import: Cannot resolve imported module `nacl`
✅ Fixed issues: none
Unchanged: 4733 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d444c4d31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # here, a first-use Discord lazy install can leave an older vulnerable | ||
| # aiohttp in place while `hermes update` considers platform.discord | ||
| # satisfied (Codex PR #6). Keep in sync with the [messaging] extra. | ||
| "platform.discord": ("discord.py==2.7.1", "brotlicffi==1.2.0.1", "aiohttp==3.14.1"), |
There was a problem hiding this comment.
When a user has aiohttp installed for another backend (for example Slack, Home Assistant, or SMS) but has never enabled Discord, active_features() still marks a feature active if any declared spec is present (tools/lazy_deps.py:541). Adding this shared transitive here means hermes update will treat platform.discord as active and can install discord.py/brotlicffi for a cold backend, violating the documented “features the user never enabled stay quiet” behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in PR #28 (fix: address Codex PR #25 follow-up issues (false feature activations + PyNaCl error)), which was merged. The active_features() logic and PyNaCl error path were updated to prevent cross-backend activation via shared transitive dependencies.
Reviewed and confirmed by Claude Code.
Generated by Claude Code
| "Install it manually (`pip install PyNaCl>=1.6.2`) to use " | ||
| "Discord voice channels." | ||
| ) | ||
| return False |
There was a problem hiding this comment.
In the missing-PyNaCl case reached by /voice join, returning False bypasses the existing dependency-specific handling in GatewayRunner._handle_voice_channel_join, which only shows the PyNaCl install guidance when join_voice_channel() raises; the false result falls through to the generic “Check bot permissions” message instead (gateway/run.py:10932-10957). This makes the new guard misleading for exactly the missing-dependency scenario it is meant to clarify.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in PR #28 (fix: address Codex PR #25 follow-up issues (false feature activations + PyNaCl error)), which was merged. The active_features() logic and PyNaCl error path were updated to prevent cross-backend activation via shared transitive dependencies.
Reviewed and confirmed by Claude Code.
Generated by Claude Code
| "terminal.modal": ("modal==1.3.4", "cbor2==6.1.2"), | ||
| "terminal.daytona": ("daytona==0.155.0",), | ||
| "terminal.vercel": ("vercel==0.5.7",), | ||
| "terminal.vercel": ("vercel==0.5.7", "cbor2==6.1.2"), |
There was a problem hiding this comment.
When a user has enabled only one of these terminal backends, cbor2 will now be present and active_features() treats either feature as active when any spec in its tuple is installed (tools/lazy_deps.py:541). That means hermes update after a Modal-only install can decide terminal.vercel is active (and install vercel), or vice versa, so cold terminal backends no longer stay quiet.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in PR #28 (fix: address Codex PR #25 follow-up issues (false feature activations + PyNaCl error)), which was merged. The active_features() logic and PyNaCl error path were updated to prevent cross-backend activation via shared transitive dependencies.
Reviewed and confirmed by Claude Code.
Generated by Claude Code
| "tool.dashboard": ( | ||
| "fastapi==0.133.1", | ||
| "uvicorn[standard]==0.41.0", | ||
| "starlette==1.3.1", |
There was a problem hiding this comment.
In installs that include the MCP extra but have never run the dashboard, starlette is already present because mcp depends on it (uv.lock:2318-2319). Since active_features() marks a lazy feature active when any spec is present, adding starlette here makes hermes update refresh tool.dashboard and install fastapi/uvicorn[standard] for a cold dashboard backend, again violating the “features the user never enabled stay quiet” contract.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in PR #28 (fix: address Codex PR #25 follow-up issues (false feature activations + PyNaCl error)), which was merged. The active_features() logic and PyNaCl error path were updated to prevent cross-backend activation via shared transitive dependencies.
Reviewed and confirmed by Claude Code.
Generated by Claude Code
active_features() was using any() over all specs, which caused false activations when a transitive package (aiohttp, cbor2, starlette) shared by multiple features was installed. For example: - aiohttp from Slack -> platform.discord falsely active - cbor2 from Modal -> terminal.vercel falsely active (and vice-versa) - starlette from MCP -> tool.dashboard falsely active Fix: check only specs[0] (the primary/unique package). By convention the first tuple element is always a package exclusive to that feature. This prevents hermes update from installing packages for features the user never enabled. Addresses Codex PR #25 comments.
Returning False from join_voice_channel() when PyNaCl is absent bypassed the PyNaCl-specific error guidance in GatewayRunner._handle_voice_channel_join (gateway/run.py:10937). The caller only shows the install hint when the call raises with "pynacl" in the error message; a False return fell through to the generic "Check bot permissions" message instead. Addresses Codex PR #25.
… + PyNaCl error) (#28) * fix(lazy_deps): use primary spec for active_features detection active_features() was using any() over all specs, which caused false activations when a transitive package (aiohttp, cbor2, starlette) shared by multiple features was installed. For example: - aiohttp from Slack -> platform.discord falsely active - cbor2 from Modal -> terminal.vercel falsely active (and vice-versa) - starlette from MCP -> tool.dashboard falsely active Fix: check only specs[0] (the primary/unique package). By convention the first tuple element is always a package exclusive to that feature. This prevents hermes update from installing packages for features the user never enabled. Addresses Codex PR #25 comments. * fix(discord): raise on missing PyNaCl instead of returning False Returning False from join_voice_channel() when PyNaCl is absent bypassed the PyNaCl-specific error guidance in GatewayRunner._handle_voice_channel_join (gateway/run.py:10937). The caller only shows the install hint when the call raises with "pynacl" in the error message; a False return fell through to the generic "Check bot permissions" message instead. Addresses Codex PR #25. * test: update pynacl guard test to expect RuntimeError The adapter now raises RuntimeError (instead of returning False) so that GatewayRunner._handle_voice_channel_join can surface the PyNaCl-specific install guidance. Update the test to match. * fix(nix): update npm lockfile hashes for tui and web * fix(nix): update npm lockfile hashes for tui and web
…nting (#67) * feat(devops): add config-integrity-watchdog skill with git-backed fingerprinting Replaces mutable .sha256 sidecar with an append-only integrity log committed to the dotfiles git repo. A process without git commit credentials cannot silently forge a fingerprint entry. - seal.py: hash config + append to log + git commit - verify.py: compare current hash against latest seal; detect log tampering - restore.py: revert from git + re-seal; backs up tampered config - PLAN.md: project plan (Linear not available) - Tests: seal->verify, seal->tamper->verify, seal->tamper->restore->verify (29 passing) Closes the 19-day recurring config hijack pattern (Event #25+). Slack: https://mfc-nyc.slack.com/archives/C0BD8QBUSJF/p1782742870774319 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh * fix(tests): add type ignore for pytest import and None guard for regex match Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh * feat(cli): add hermes config seal/verify/restore commands Integrates config-integrity-watchdog into the Hermes CLI so users can seal, verify, and restore config integrity without remembering script paths. Calls core logic shared with the standalone scripts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh * fix(cli): fallback import path, move PLAN.md to .plans/, add restore caveat - _import_core() now tries ~/.hermes/skills first, then repo-relative skills/ as fallback for pre-sync usage; prints clear error + exits 1 if skill not found in either location - Move PLAN.md to .plans/config-integrity-watchdog.md - Append git-HEAD caveat to restore.py module docstring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh --------- Co-authored-by: Claude <noreply@anthropic.com>
1 participant
Automated follow-up for chatgpt-codex-connector comments on merged PRs #6 and #10.
tools/lazy_deps.py (PR #6): Lazy install pins were stale after the Dependabot audit — the messaging extras were updated but the lazy-install path (used by first-use installs and
hermes update) still pointed at old vulnerable versions:plugins/platforms/discord/adapter.py (PR #10): The
voiceextra was stripped of PyNaCl (vulnerable pin), but /voice join was still registered and would crash at runtime. Added a PyNaCl availability guard in join_voice_channel that fails fast with a logged warning instead of an opaque missing-module error.PR #3 website comments already fixed in main: serialize-javascript override at 7.0.5, all @docusaurus/* aligned at 3.10.1.
Verification: pytest (157 passed, 2 pre-existing warnings). Regression tests added for the voice guard.
🤖 Generated by auto-fix-codex-pr-review-recommendations automation. Draft PR — no auto-merge.