-
Notifications
You must be signed in to change notification settings - Fork 0
Resolve postponed handler annotations during signature validation #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
39e6bff
1a63e76
58ac724
ece7a14
84d93b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| name: AI Issue Triage | ||
|
|
||
| on: | ||
| issues: | ||
| types: [opened] | ||
| discussion: | ||
| types: [created] | ||
| workflow_dispatch: | ||
| inputs: | ||
| reference: | ||
| description: 'Reference (e.g., "issue:123" or "discussion:456")' | ||
| required: true | ||
| type: string | ||
| live_mode: | ||
| description: 'Enable live mode' | ||
| required: false | ||
| type: boolean | ||
| default: false | ||
|
|
||
| concurrency: | ||
| group: triage-${{ github.event.issue.number || github.event.discussion.number || github.event.inputs.reference }} | ||
| cancel-in-progress: false | ||
|
|
||
| permissions: | ||
| issues: write | ||
| discussions: write | ||
| contents: read | ||
|
|
||
| jobs: | ||
| security-check: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| should_run: ${{ steps.check.outputs.should_run }} | ||
| reference: ${{ steps.check.outputs.reference }} | ||
| steps: | ||
| - name: Validate | ||
| id: check | ||
| run: | | ||
| if [[ "${{ github.actor }}" == *"[bot]"* ]] || [[ "${{ github.event.repository.fork }}" == "true" ]]; then | ||
| echo "should_run=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [[ "${{ github.event_name }}" == "issues" ]]; then | ||
| REF="issue:${{ github.event.issue.number }}" | ||
| elif [[ "${{ github.event_name }}" == "discussion" ]]; then | ||
| REF="discussion:${{ github.event.discussion.number }}" | ||
| elif [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then | ||
| REF="${{ github.event.inputs.reference }}" | ||
| else | ||
| echo "should_run=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| if ! [[ "$REF" =~ ^(issue|discussion):[0-9]+$ ]]; then | ||
| echo "should_run=false" >> $GITHUB_OUTPUT | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "should_run=true" >> $GITHUB_OUTPUT | ||
| echo "reference=$REF" >> $GITHUB_OUTPUT | ||
|
|
||
| triage: | ||
| needs: security-check | ||
| if: needs.security-check.outputs.should_run == 'true' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| env: | ||
| UV_CACHE_DIR: /tmp/.uv-cache | ||
| TRIAGE_LIVE: ${{ github.event.inputs.live_mode || 'false' }} | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| sparse-checkout: | | ||
| python | ||
| .github/actions | ||
| sparse-checkout-cone-mode: false | ||
|
|
||
| - name: Setup | ||
| run: git clone https://${{ secrets.TRIAGE_BOT_REPO_TOKEN }}@github.com/${{ vars.TRIAGE_BOT_REPO }}.git triage-bot | ||
| env: | ||
| GIT_TERMINAL_PROMPT: 0 | ||
|
|
||
| - name: Set up uv | ||
| uses: astral-sh/setup-uv@v6 | ||
| with: | ||
| enable-cache: true | ||
| cache-dependency-glob: "python/**/uv.lock" | ||
|
|
||
| - name: Install | ||
| working-directory: python | ||
| run: uv sync --package agent-framework-core --extra openai | ||
|
|
||
| - name: Pull GitHub MCP server image | ||
| run: docker pull ghcr.io/github/github-mcp-server | ||
|
|
||
| - name: Run | ||
| working-directory: python | ||
| env: | ||
| GITHUB_PERSONAL_ACCESS_TOKEN: ${{ secrets.GH_ACTIONS_PR_WRITE }} | ||
| OPENAI_API_KEY: ${{ secrets.OPENAI__APIKEY }} | ||
| AZURE_OPENAI_ENDPOINT: ${{ secrets.AZURE_OPENAI_ENDPOINT }} | ||
| AZURE_OPENAI_API_VERSION: ${{ secrets.AZURE_OPENAI_API_VERSION }} | ||
| TRIAGE_REFERENCE: ${{ needs.security-check.outputs.reference }} | ||
| TRIAGE_BOT_PATH: ${{ github.workspace }}/triage-bot | ||
| run: | | ||
| echo "::add-mask::$GITHUB_PERSONAL_ACCESS_TOKEN" | ||
| echo "::add-mask::$OPENAI_API_KEY" | ||
| uv run python $TRIAGE_BOT_PATH/run.py | ||
|
|
||
| - name: Summary | ||
| if: always() | ||
| run: | | ||
| echo "## Triage Summary" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **Reference:** ${{ needs.security-check.outputs.reference }}" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **Status:** ${{ job.status }}" >> $GITHUB_STEP_SUMMARY |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| # Example GitHub Action Workflow for Docs Update Bot | ||
| # | ||
| # Place this file in the PUBLIC repo at: | ||
| # .github/workflows/docs-update.yml | ||
| # | ||
| # Required Secrets: | ||
| # - DOCS_BOT_REPO_TOKEN: PAT with access to clone the private docs-bot repo | ||
| # - GH_ACTIONS_PR_WRITE: PAT with write access to the docs repo | ||
| # - OPENAI__APIKEY: OpenAI API key (or use Azure OpenAI secrets) | ||
| # | ||
| # Required Variables: | ||
| # - DOCS_BOT_REPO: The private repo containing the docs-bot code (e.g., "your-org/docs-bot") | ||
|
|
||
| name: Docs Update on PR Merge | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [closed] | ||
| branches: [main] | ||
|
|
||
| # Allow manual trigger for testing | ||
| workflow_dispatch: | ||
| inputs: | ||
| pr_number: | ||
| description: 'PR number to process' | ||
| required: true | ||
| type: number | ||
|
|
||
| concurrency: | ||
| group: docs-update-${{ github.event.pull_request.number || github.event.inputs.pr_number }} | ||
| cancel-in-progress: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| jobs: | ||
| # Security check - skip for bots and forks | ||
| security-check: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| should_run: ${{ steps.check.outputs.should_run }} | ||
| pr_number: ${{ steps.check.outputs.pr_number }} | ||
| pr_title: ${{ steps.check.outputs.pr_title }} | ||
| pr_url: ${{ steps.check.outputs.pr_url }} | ||
| merge_sha: ${{ steps.check.outputs.merge_sha }} | ||
| steps: | ||
| - name: Validate | ||
| id: check | ||
| run: | | ||
| # Skip for bots and forks | ||
| if [[ "${{ github.actor }}" == *"[bot]"* ]] || [[ "${{ github.event.repository.fork }}" == "true" ]]; then | ||
| echo "should_run=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| # For PR merge events | ||
| if [[ "${{ github.event_name }}" == "pull_request" ]]; then | ||
| # Only run for merged PRs | ||
| if [[ "${{ github.event.pull_request.merged }}" != "true" ]]; then | ||
| echo "should_run=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "pr_number=${{ github.event.pull_request.number }}" >> $GITHUB_OUTPUT | ||
| echo "pr_title=${{ github.event.pull_request.title }}" >> $GITHUB_OUTPUT | ||
| echo "pr_url=${{ github.event.pull_request.html_url }}" >> $GITHUB_OUTPUT | ||
| echo "merge_sha=${{ github.event.pull_request.merge_commit_sha }}" >> $GITHUB_OUTPUT | ||
|
|
||
| # For manual dispatch | ||
| elif [[ "${{ github.event_name }}" == "workflow_dispatch" ]]; then | ||
| echo "pr_number=${{ github.event.inputs.pr_number }}" >> $GITHUB_OUTPUT | ||
| # For manual dispatch, we need to fetch PR details | ||
| echo "pr_title=Manual trigger for PR #${{ github.event.inputs.pr_number }}" >> $GITHUB_OUTPUT | ||
| echo "pr_url=${{ github.server_url }}/${{ github.repository }}/pull/${{ github.event.inputs.pr_number }}" >> $GITHUB_OUTPUT | ||
| echo "merge_sha=" >> $GITHUB_OUTPUT | ||
|
|
||
| else | ||
| echo "should_run=false" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "should_run=true" >> $GITHUB_OUTPUT | ||
|
|
||
| docs-update: | ||
| needs: security-check | ||
| if: needs.security-check.outputs.should_run == 'true' | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| env: | ||
| UV_CACHE_DIR: /tmp/.uv-cache | ||
|
|
||
| steps: | ||
| - name: Checkout source repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| sparse-checkout: | | ||
| python | ||
| sparse-checkout-cone-mode: false | ||
|
|
||
| - name: Clone docs-bot | ||
| run: git clone https://${{ secrets.DOCS_BOT_REPO_TOKEN }}@github.com/${{ vars.DOCS_BOT_REPO }}.git docs-bot | ||
| env: | ||
| GIT_TERMINAL_PROMPT: 0 | ||
|
|
||
| - name: Set up uv | ||
| uses: astral-sh/setup-uv@v6 | ||
| with: | ||
| enable-cache: true | ||
| cache-dependency-glob: "python/**/uv.lock" | ||
|
|
||
| - name: Install dependencies | ||
| working-directory: python | ||
| run: uv sync --package agent-framework-core --extra openai | ||
|
|
||
| - name: Pull GitHub MCP server image | ||
| run: docker pull ghcr.io/github/github-mcp-server | ||
|
|
||
| - name: Run docs-bot | ||
| working-directory: python | ||
| env: | ||
| # GitHub tokens | ||
| GITHUB_PERSONAL_ACCESS_TOKEN: ${{ secrets.GH_ACTIONS_PR_WRITE }} | ||
|
|
||
| # LLM configuration | ||
| OPENAI_API_KEY: ${{ secrets.OPENAI__APIKEY }} | ||
| # Uncomment for Azure OpenAI: | ||
| # AZURE_OPENAI_ENDPOINT: ${{ secrets.AZURE_OPENAI_ENDPOINT }} | ||
| # AZURE_OPENAI_API_VERSION: ${{ secrets.AZURE_OPENAI_API_VERSION }} | ||
|
|
||
| # PR information | ||
| PR_NUMBER: ${{ needs.security-check.outputs.pr_number }} | ||
| PR_TITLE: ${{ needs.security-check.outputs.pr_title }} | ||
| PR_URL: ${{ needs.security-check.outputs.pr_url }} | ||
| MERGE_SHA: ${{ needs.security-check.outputs.merge_sha }} | ||
|
|
||
| # Repository configuration | ||
| SOURCE_REPO: ${{ github.repository }} | ||
| DOCS_REPO: microsoftdocs/semantic-kernel-pr | ||
|
|
||
| # Workflow settings | ||
| CONFIDENCE_THRESHOLD_HIGH: "0.7" | ||
| CONFIDENCE_THRESHOLD_LOW: "0.4" | ||
|
|
||
| # Path to docs-bot | ||
| DOCS_BOT_PATH: ${{ github.workspace }}/docs-bot | ||
| run: | | ||
| echo "::add-mask::$GITHUB_PERSONAL_ACCESS_TOKEN" | ||
| echo "::add-mask::$OPENAI_API_KEY" | ||
| uv run python $DOCS_BOT_PATH/run.py | ||
|
|
||
| - name: Summary | ||
| if: always() | ||
| run: | | ||
| echo "## Docs Update Summary" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **PR Number:** ${{ needs.security-check.outputs.pr_number }}" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **PR Title:** ${{ needs.security-check.outputs.pr_title }}" >> $GITHUB_STEP_SUMMARY | ||
| echo "- **Status:** ${{ job.status }}" >> $GITHUB_STEP_SUMMARY |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| import inspect | ||
| import logging | ||
| import types | ||
| import typing | ||
| from collections.abc import Awaitable, Callable | ||
| from typing import Any, TypeVar, overload | ||
|
|
||
|
|
@@ -712,30 +713,57 @@ def _validate_handler_signature( | |
| signature = inspect.signature(func) | ||
| params = list(signature.parameters.values()) | ||
|
|
||
| type_hints: dict[str, Any] = {} | ||
| type_hint_error: Exception | None = None | ||
| try: | ||
| type_hints = typing.get_type_hints( | ||
| func, | ||
| globalns=getattr(func, "__globals__", None), | ||
| localns=None, | ||
| include_extras=True, | ||
| ) | ||
| except Exception as exc: | ||
| type_hint_error = exc | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: Catching broad |
||
|
|
||
| def _get_annotation(param: inspect.Parameter) -> Any: | ||
| return type_hints.get(param.name, param.annotation) | ||
|
|
||
| expected_counts = 3 # self, message, ctx | ||
| param_description = "(self, message: T, ctx: WorkflowContext[U, V])" | ||
| if len(params) != expected_counts: | ||
| raise ValueError(f"Handler {func.__name__} must have {param_description}. Got {len(params)} parameters.") | ||
|
|
||
| # Check message parameter has type annotation (unless skipped) | ||
| message_param = params[1] | ||
| if not skip_message_annotation and message_param.annotation == inspect.Parameter.empty: | ||
| message_annotation = _get_annotation(message_param) | ||
| if not skip_message_annotation and message_annotation == inspect.Parameter.empty: | ||
| raise ValueError(f"Handler {func.__name__} must have a type annotation for the message parameter") | ||
| if type_hint_error and not skip_message_annotation and isinstance(message_annotation, str): | ||
| raise ValueError( | ||
| f"Handler {func.__name__} has an unresolved message annotation for parameter " | ||
| f"'{message_param.name}'. Ensure all referenced types are available for type resolution." | ||
| ) from type_hint_error | ||
|
|
||
| # Validate ctx parameter is WorkflowContext and extract type args | ||
| ctx_param = params[2] | ||
| if skip_message_annotation and ctx_param.annotation == inspect.Parameter.empty: | ||
| ctx_annotation = _get_annotation(ctx_param) | ||
| if skip_message_annotation and ctx_annotation == inspect.Parameter.empty: | ||
| # When explicit types are provided via @handler(input=..., output=...), | ||
| # the ctx parameter doesn't need a type annotation - types come from the decorator. | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: |
||
| output_types: list[type[Any] | types.UnionType] = [] | ||
| workflow_output_types: list[type[Any] | types.UnionType] = [] | ||
| else: | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: |
||
| if type_hint_error and isinstance(ctx_annotation, str): | ||
| raise ValueError( | ||
| f"Handler {func.__name__} has an unresolved WorkflowContext annotation for parameter " | ||
| f"'{ctx_param.name}'. Ensure all referenced types are available for type resolution." | ||
| ) from type_hint_error | ||
| output_types, workflow_output_types = validate_workflow_context_annotation( | ||
| ctx_param.annotation, f"parameter '{ctx_param.name}'", "Handler" | ||
| ctx_annotation, f"parameter '{ctx_param.name}'", "Handler" | ||
| ) | ||
|
|
||
| message_type = message_param.annotation if message_param.annotation != inspect.Parameter.empty else None | ||
| ctx_annotation = ctx_param.annotation | ||
| message_type = message_annotation if message_annotation != inspect.Parameter.empty else None | ||
| ctx_annotation = ctx_annotation | ||
|
|
||
| return message_type, ctx_annotation, output_types, workflow_output_types | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from agent_framework._workflows._executor import Executor, handler | ||
| from agent_framework._workflows._workflow_context import WorkflowContext | ||
|
|
||
|
|
||
| class OutputTypeA: | ||
| pass | ||
|
|
||
|
|
||
| class OutputTypeB: | ||
| pass | ||
|
|
||
|
|
||
| class PostponedAnnotationExecutor(Executor): | ||
| @handler | ||
| async def handle(self, message: str, ctx: WorkflowContext[OutputTypeA, OutputTypeB]) -> None: | ||
| return None | ||
|
|
||
|
|
||
| def test_postponed_annotations_are_resolved() -> None: | ||
| executor = PostponedAnnotationExecutor(id="postponed") | ||
| assert set(executor.output_types) == {OutputTypeA} | ||
| assert set(executor.workflow_output_types) == {OutputTypeB} | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: Test only covers the happy-path. Add at least one failure-mode test where
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking (tests): Add failure-mode tests that force |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: Catching broad
Exceptionfromtyping.get_type_hints()can mask real resolution/runtime issues and then continue with unresolved/incorrect annotations. Narrow the exceptions and/or fail fast when required annotations can’t be resolved.