Skip to content

fix(python): correct PermissionHandler.approve_all type annotations#618

Merged
SteveSandersonMS merged 1 commit intogithub:mainfrom
giulio-leone:fix/issue-612-permission-handler-return-type
Mar 2, 2026
Merged

fix(python): correct PermissionHandler.approve_all type annotations#618
SteveSandersonMS merged 1 commit intogithub:mainfrom
giulio-leone:fix/issue-612-permission-handler-return-type

Conversation

@giulio-leone
Copy link
Contributor

Summary

Fixes #612

PermissionHandler.approve_all used Any parameter types and returned a plain dict, conflicting with the _PermissionHandlerFn signature:

_PermissionHandlerFn = Callable[
    [PermissionRequest, dict[str, str]],
    PermissionRequestResult | Awaitable[PermissionRequestResult],
]

Changes

  • Replace parameter types: AnyPermissionRequest, dict[str, str]
  • Replace return type: dictPermissionRequestResult
  • Use PermissionRequestResult() constructor instead of dict literal

Before

def approve_all(request: Any, invocation: Any) -> dict:
    return {"kind": "approved"}

After

def approve_all(
    request: PermissionRequest, invocation: dict[str, str]
) -> PermissionRequestResult:
    return PermissionRequestResult(kind="approved")

Tests

  • 15 unit tests passed
  • ruff check clean
  • 2 consecutive review runs with 0 errors

The approve_all static method used untyped parameters (Any) and returned
a plain dict, conflicting with the _PermissionHandlerFn signature that
expects (PermissionRequest, dict[str, str]) -> PermissionRequestResult.

- Replace parameter types: Any → PermissionRequest, dict[str, str]
- Replace return type: dict → PermissionRequestResult
- Use PermissionRequestResult() constructor instead of dict literal

Fixes github#612

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 1, 2026 04:27
@giulio-leone giulio-leone requested a review from a team as a code owner March 1, 2026 04:27
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Python SDK’s built-in PermissionHandler.approve_all helper to have type annotations consistent with the _PermissionHandlerFn callback signature expected by SessionConfig.on_permission_request, resolving the type-checker mismatch reported in #612.

Changes:

  • Tighten approve_all parameter types from Any to PermissionRequest and dict[str, str].
  • Update approve_all return type from dict to PermissionRequestResult.
  • Construct the result using PermissionRequestResult(kind="approved") rather than a plain dict literal.

@giulio-leone
Copy link
Contributor Author

Thanks for the review — accurate summary of the type annotation improvement.

@SteveSandersonMS SteveSandersonMS merged commit b9f746a into github:main Mar 2, 2026
19 of 20 checks passed
@SteveSandersonMS
Copy link
Contributor

Thanks @giulio-leone!

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.

PermissionHandler.approve_all has incorrect return type annotation

3 participants