Skip to content

fix(plugins): clean up standalone sender ty warnings - #24210

Open
GodsBoy wants to merge 3 commits into
NousResearch:mainfrom
GodsBoy:fix/pr-22461-ty-warnings
Open

GodsBoy wants to merge 3 commits into
NousResearch:mainfrom
GodsBoy:fix/pr-22461-ty-warnings

Conversation

@GodsBoy

@GodsBoy GodsBoy commented May 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Rebase the stale fork branch onto current main while preserving the original focused typing cleanups
  • Use importlib for the optional aiohttp import in Google Chat standalone send without changing its runtime behaviour
  • Narrow the optional Google Chat service_account module in both standalone-send credential mock tests
  • Annotate IRC setup defaults so ty does not infer a heterogeneous value union for PlatformEntry keyword arguments
  • Record the review follow-up and verification contract in docs/plans/2026-07-13-001-fix-pr-24210-review-followup-plan.md

Verification

  • scripts/run_tests.sh tests/gateway/test_google_chat.py tests/hermes_cli/test_setup_irc.py - 168 passed
  • ruff check . - passed
  • python scripts/check-windows-footguns.py --all - passed
  • python -m compileall -q plugins/platforms/google_chat/adapter.py tests/gateway/test_google_chat.py tests/hermes_cli/test_setup_irc.py - passed
  • CI-style Ruff and ty diff against the rebased origin/main - no new Ruff issues and 22 fewer ty diagnostics
  • Full local suite - 40,402 passed and 23 failed in unrelated, unchanged areas; the corresponding failure set reproduced on current main, apart from one timing-sensitive cache invalidation test in unchanged code

Addresses the advisory ty diff reported on #22461: #22461 (comment)

@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels May 12, 2026
@GodsBoy

GodsBoy commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

CI triage note: the current red checks appear to be inherited from main, not caused by this PR.

This PR only changes:

  • plugins/platforms/google_chat/adapter.py
  • tests/gateway/test_google_chat.py
  • tests/hermes_cli/test_setup_irc.py

Relevant PR-specific checks passed:

  • ruff + ty diff ✅
  • ruff enforcement (blocking) ✅
  • targeted local tests for the touched Google Chat / IRC setup files passed before opening this PR

Comparison against latest main at 99ad2d137:

I am intentionally not broadening this cleanup PR to fix unrelated CI/main breakage. The actual cleanup in this PR is scoped to the advisory ty warnings reported on the already-merged #22461: #22461 (comment)

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused ty cleanup. The three original source patterns remain on current main, including the optional aiohttp import at plugins/platforms/google_chat/adapter.py:3250 and the heterogeneous PlatformEntry(**defaults) setup at tests/hermes_cli/test_setup_irc.py:19-39.

Problems

  • The Google Chat test narrowing only covers the API-failure test. Current main's preceding refresh-success test still dereferences _gc_mod.service_account.Credentials at tests/gateway/test_google_chat.py:2858, :2859, and :2874, leaving the same potentially-None typing pattern outside this PR's hunk.

Suggested changes

  • Apply the same local service_account binding and non-None assertion to that refresh-success test before salvaging this cleanup.

Automated hermes-sweeper review.

Comment thread tests/gateway/test_google_chat.py
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@GodsBoy
GodsBoy force-pushed the fix/pr-22461-ty-warnings branch from 54d7258 to 7caeba6 Compare July 13, 2026 04:51

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants