Skip to content

Collapse GitHub integration into one injectable public client and remove hidden factories (closes #311)#442

Merged
FidoCanCode merged 9 commits into
mainfrom
consolidate-github-client
Apr 14, 2026
Merged

Collapse GitHub integration into one injectable public client and remove hidden factories (closes #311)#442
FidoCanCode merged 9 commits into
mainfrom
consolidate-github-client

Conversation

@FidoCanCode
Copy link
Copy Markdown
Owner

@FidoCanCode FidoCanCode commented Apr 14, 2026

Collapses the parallel GH and GitHub clients into a single injectable GitHub collaborator, removing cached factories and hidden defaults across events, the webhook handler, the registry, and CLI entry points. Tests now construct fakes directly instead of patching module globals.

Fixes #311.


Work queue

Completed (6)
  • Collapse GH and GitHub into a single injectable GitHub client
  • Remove _get_gh and get_github cached factories, require explicit construction
  • Inject GitHub collaborator into events.py functions, drop _gh keyword fallback
  • Replace WebhookHandler._fn_get_github with an injected GitHub instance
  • Drop hidden GitHub defaults from registry, cli, gh_status, and main entry points
  • Update tests to fake GitHub by construction instead of patching module globals

GH was a forwarding-only layer: GitHub held a GH instance and
delegated every method call to it verbatim, adding no behaviour.
Merge GH's body directly into GitHub so there is one public client.

- GitHub.__init__ now creates _s and sets auth headers itself
- get_default_branch absorbs the combined get_repo_info + REST call
  that the old GitHub facade provided; GH's (repo: str) overload drops
- _get_gh cached factory removed (it was GH-specific and unused outside
  tests); get_github still exists for the next refactor step
- TestGHClass renamed TestGitHubClass; delegates-only tests removed;
  get_review_threads and resolve_thread gain direct coverage
@FidoCanCode FidoCanCode force-pushed the consolidate-github-client branch from a018081 to e46f153 Compare April 14, 2026 03:47
get_github was a @functools.cache singleton: one shared GitHub instance
per token, silently shared across threads and requests.  Remove it.

- kennel/github.py: drop get_github and functools import
- events.py: all seven _gh-or-factory fallbacks now use GitHub() directly
- gh_status.py: _get_github default changes from get_github to GitHub
  (same injectable slot, no caching)
- Tests updated: TestGetGithub removed, test_gh_none_uses_get_github
  now patches kennel.events.GitHub instead of the deleted factory
Comment thread kennel/events.py Outdated
… fallback

All events.py public functions now accept `gh: GitHub` as a required positional
parameter instead of an optional `_gh=None` keyword with an internal fallback.
server.py constructs `gh = type(self)._fn_get_github()` once at the top of
`_process_action` and threads it through all call sites.
Drop the _fn_get_github factory class attribute and replace it with a gh
instance attribute (GitHub()) set once at class definition time and again in
run(). _process_action and _signal_action_error now use self.gh directly
instead of calling a factory per request.
Resolves conflicts from #449 (issue picker) landing on main:
- kennel/github.py: GH class retained (collapsed-client plan),
  dropped the now-obsolete GitHub wrapper class from main.  New
  methods from #449 (find_issues with cursor pagination,
  get_sub_issues, get_issue_node, add_assignee) are already on GH.
- tests/test_github.py: dropped wrapper delegation tests for the
  removed GitHub class; kept the direct GH-class tests for the
  new methods.
Pass a shared GitHub instance through make_registry, Cmd, and
set_gh_status rather than constructing one lazily inside each call
site.  This eliminates the eager GitHub() at WebhookHandler class
body scope that was calling gh auth token at import time and
breaking CI collection.
… auth

GitHub() called _gh_token() which tried gh CLI in CI (no auth = boom).
Add _GitHub=GitHub default param to server.run(), cli.main(), and
gh_status.main(), then pass _GitHub=MagicMock in all affected tests.
The task branch in main() called task_main(args[1:]) with no _GitHub
argument, so the default-argument-bound GitHub class was used at cli.py
load time. Patching kennel.github.GitHub after import had no effect.

Import GitHub inside the function body (matching the sync-tasks branch
pattern) and forward it via the _GitHub kwarg so test patches intercept
the real constructor.
…e globals

Add _GitHub injectable to worker.run() and main.main() so tests can pass a
mock class directly rather than patching kennel.worker.GitHub or
kennel.github.GitHub at the module level.
@FidoCanCode FidoCanCode marked this pull request as ready for review April 14, 2026 06:48
@FidoCanCode FidoCanCode requested a review from rhencke April 14, 2026 06:48
@FidoCanCode FidoCanCode merged commit b2ae112 into main Apr 14, 2026
2 checks passed
@FidoCanCode FidoCanCode deleted the consolidate-github-client branch April 14, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collapse GitHub integration into one injectable public client and remove hidden factories

2 participants