Skip to content

Fix PermissionHandler import path for github-copilot-sdk 0.3.0#146

Merged
0xba1a merged 2 commits into
mainfrom
copilot/fix-import-path-permissionhandler
May 19, 2026
Merged

Fix PermissionHandler import path for github-copilot-sdk 0.3.0#146
0xba1a merged 2 commits into
mainfrom
copilot/fix-import-path-permissionhandler

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

Problem

The claim in the issue is correct.

CopilotBot.__init__ did:

try:
    from copilot import CopilotClient, ExternalServerConfig
    from copilot.types import PermissionHandler
except ImportError:
    raise ImportError("CopilotBot requires the github-copilot-sdk package. ...")

In github-copilot-sdk 0.3.0 (released April 24, after microbots 0.0.17 on April 10), PermissionHandler was moved from copilot.types to copilot.session, so users with the new SDK installed hit the inner ImportError and saw a misleading "github-copilot-sdk is not installed" message. Verified against the SDK 0.3.0 PyPI README which documents from copilot.session import PermissionHandler.

Fix

In src/microbots/bot/CopilotBot.py:

  • Split into two try/except blocks so the "not installed" message is only raised when the SDK isn't installed.
  • Try copilot.session first (SDK ≥ 0.3.0), fall back to copilot.types (older SDKs).
  • If neither location works, raise a distinct, accurate error pointing the user to upgrade.

Tests

  • Verify the claim against github-copilot-sdk 0.3.0
  • Update CopilotBot.__init__ to import PermissionHandler from copilot.session first, falling back to copilot.types
  • Tighten the outer try/except so a missing CopilotClient/ExternalServerConfig is what triggers the "not installed" message
  • Original mock-update test (mocks register PermissionHandler on both copilot.session and copilot.types)
  • Add unit tests covering 100% of the new lines:
    • PermissionHandler is loaded from copilot.session when present (SDK ≥ 0.3.0 path)
    • Falls back to copilot.types when copilot.session lacks the attribute (old-SDK path)
    • Raises a distinct ImportError when neither module exposes PermissionHandler
  • Add two integration tests exercising CopilotBot.run() with each SDK layout simulated (new TestCopilotBotSDKLayoutIntegration)
  • Validate — pytest -m "unit or integration" reports 65 passed, 2 skipped (the 2 skipped require real Docker+CLI+auth), with 100% line coverage on src/microbots/bot/CopilotBot.py

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.04%. Comparing base (d41a0a9) to head (d4a23d8).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #146      +/-   ##
==========================================
+ Coverage   64.95%   65.04%   +0.08%     
==========================================
  Files          34       34              
  Lines        2374     2380       +6     
==========================================
+ Hits         1542     1548       +6     
  Misses        832      832              
Flag Coverage Δ
integration 37.43% <71.42%> (+2.93%) ⬆️
ollama_local 31.97% <0.00%> (-0.25%) ⬇️
slow-browser 26.76% <0.00%> (-0.07%) ⬇️
slow-other 37.94% <0.00%> (-0.61%) ⬇️
unit 59.28% <100.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/microbots/bot/CopilotBot.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot finished work on behalf of 0xba1a May 19, 2026 11:19
@0xba1a 0xba1a merged commit 11f8ba3 into main May 19, 2026
7 checks passed
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.

4 participants