Fix handler signature validation for future annotations by moonbox3 · Pull Request #11 · moonbox3/agent-framework

moonbox3 · 2026-02-18T00:34:36Z

Summary

resolve handler annotations via typing.get_type_hints and surface invalid annotation errors
reuse resolved ctx annotation in explicit @handler mode
add regression tests for future annotations, explicit handler types, and forward references

Testing

python/packages/core/tests/workflow/test_executor_future_annotations.py

## Summary - resolve handler annotations via `typing.get_type_hints` before validation - reuse resolved ctx annotation in explicit @handler mode - add regression test for future annotations on @handler methods ## Testing - `python/packages/core/tests/workflow/test_executor_future_annotations.py` Fixes #1

moonbox3

Automated PR Review Round 1: CHANGES_REQUESTED

Issue: #1
The core change to resolve handler annotations via typing.get_type_hints addresses the reported from __future__ import annotations/PEP 563 failure and adds a regression test, but the PR also introduces unrelated GitHub Actions workflows with high-risk secret handling and supply-chain exposure. Additionally, _validate_handler_signature currently swallows all get_type_hints errors, which can silently mask real annotation/forward-ref problems and reduce validation reliability. As-is, this should not be merged until the workflow security issues are removed/hardened and the handler validation/test coverage gaps are addressed.

Blocking issues (3):

Remove or substantially harden the newly-added GitHub Actions workflows that clone/run external code with secrets (including secrets embedded in clone URLs) and unpinned dependenci
In python/packages/core/agent_framework/_workflows/_executor.py, _validate_handler_signature catches Exception around typing.get_type_hints and falls back to {} with no s
Add regression coverage for the explicit decorator mode (@handler(input=..., output=...) / skip_message_annotation=True) under from __future__ import annotations, since this

Suggestions (4):

Add a targeted forward-reference test (e.g., message: "MyMessage" or a string-annotated WorkflowContext[...]) to ensure get_type_hints resolution is actually exercised.
If keeping a fallback when get_type_hints fails, emit at least debug/warn logging with func.__qualname__ and the exception to aid diagnosis.
Consider passing appropriate globalns/localns into get_type_hints to improve forward-ref resolution for nested/local definitions.
+1 more

moonbox3 · 2026-02-18T00:35:21Z

python/packages/core/agent_framework/_workflows/_executor.py

+            include_extras=True,
+        )
+    except Exception:
+        hints = {}


Blocking (reliability): except Exception: hints = {} silently ignores typing.get_type_hints failures (e.g., NameError on forward refs), potentially allowing incorrect handler validation. Catch narrower exceptions and/or raise/log a clear ValueError so annotation issues are actionable.

Automated status (r1): queued for remediation in this pass.

Automated status (r2): queued for remediation in this pass.

moonbox3 · 2026-02-18T00:35:21Z

python/packages/core/tests/workflow/test_executor_future_annotations.py

+
+    executor = FutureExecutor(id="future")
+
+    assert dict[str, Any] in executor._handlers


Blocking (coverage): add a regression test for explicit @handler(input=..., output=...) mode under from __future__ import annotations, since that path now depends on _validate_handler_signature returning ctx_annotation.

Automated status (r1): queued for remediation in this pass.

Automated status (r2): queued for remediation in this pass.

moonbox3 · 2026-02-18T00:35:21Z

python/packages/core/tests/workflow/test_executor_future_annotations.py

+            pass
+
+    executor = FutureExecutor(id="future")
+


Suggestion: the test asserts against private internals (_handlers, _handler_spec) and may be brittle; prefer asserting observable behavior (e.g., registration/execution succeeds and output types match expectations).

Automated status (r1): queued for remediation in this pass.

Automated status (r2): queued for remediation in this pass.

## Summary - resolve handler annotations via `typing.get_type_hints` and surface invalid annotation errors - reuse resolved ctx annotation in explicit @handler mode - add regression tests for future annotations, explicit handler types, and forward references ## Testing - `python/packages/core/tests/workflow/test_executor_future_annotations.py` Fixes #1

moonbox3

Automated PR Review Round 2: CHANGES_REQUESTED

Issue: #1
The annotation-resolution fix using typing.get_type_hints is on the right track for from __future__ import annotations, but the PR currently bundles high-risk GitHub Actions workflows that execute external code with secrets, which should not ship with this change. In the core code, get_type_hints failures are silently swallowed and explicit-mode ctx validation can mis-handle unresolved string annotations, risking unexpected runtime errors. Tests cover the scenario but are brittle because they assert private internals instead of user-observable behavior.

Blocking issues (4):

Remove or move the newly added GitHub Actions workflows that clone/execute external code with high-privilege secrets/tokens (supply-chain and secret-exfiltration risk) since it is
In python/packages/core/agent_framework/_workflows/_executor.py, do not swallow typing.get_type_hints exceptions silently: at minimum log (debug/warn) the exception with `func.
Fix _validate_handler_signature explicit-mode ctx handling: under future annotations ctx_annotation can be a string; ensure ctx_annotation is resolved to an actual type bef
Rewrite/adjust the new tests so they assert public/observable behavior rather than private internals (executor._handlers, handler_func._handler_spec), to keep the regression te

Suggestions (3):

Add a negative test for unresolvable forward refs (e.g., missing name) to validate the chosen behavior (clear error or logged fallback) and ensure it’s actionable.
Consider improving get_type_hints resolution by providing an appropriate localns (or documenting limitations) when nested/local handler definitions are expected.
Add coverage ensuring explicit @handler(input=..., output=...) still enforces ctx to be a WorkflowContext when annotated, and fails clearly for non-WorkflowContext types.

moonbox3 · 2026-02-18T00:39:57Z

python/packages/core/agent_framework/_workflows/_executor.py

+            localns=None,
+            include_extras=True,
+        )
+    except Exception:


Blocking/reliability: catching Exception from typing.get_type_hints and silently falling back to {} masks forward-ref/future-annotations failures and can change behavior without visibility. Log (debug/warn) with func.__qualname__ + exception details, and consider raising a clear error when resolution is required.

Automated status (r2): queued for remediation in this pass.

moonbox3 · 2026-02-18T00:39:57Z

python/packages/core/agent_framework/_workflows/_executor.py

-    if skip_message_annotation and ctx_param.annotation == inspect.Parameter.empty:
+    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.


Blocking/correctness: in explicit @handler(...) mode, ctx_annotation may be a string under future annotations; treating it as a real annotation will call validate_workflow_context_annotation on str and can raise unexpectedly. Resolve ctx_annotation via get_type_hints (or treat unresolved strings as absent/unresolved) before validation.

Automated status (r2): queued for remediation in this pass.

moonbox3 · 2026-02-18T00:39:57Z

python/packages/core/tests/workflow/test_executor_future_annotations.py

+            pass
+
+    executor = FutureExecutor(id="future")
+


Blocking: this test asserts on private internals (executor._handlers, _handler_spec), making it brittle. Prefer asserting observable behavior (registration via public API, executor.output_types, and/or a minimal execution path) to keep the regression test stable.

Automated status (r2): queued for remediation in this pass.

moonbox3 · 2026-02-18T00:39:57Z

python/packages/core/tests/workflow/test_executor_future_annotations.py

+        async def handle(self, message, ctx: WorkflowContext[int]) -> None:  # type: ignore[no-untyped-def]
+            pass
+
+    executor = ExplicitFutureExecutor(id="explicit_future")


This assertion on _handler_spec["ctx_annotation"] is brittle; if possible assert the resolved types through public/observable surfaces (e.g., output types or successful execution) rather than internal spec structure.

Automated status (r2): queued for remediation in this pass.

Add test_deepcopy_shallow_copy_fields_override_default_exclude to verify that a field in both DEFAULT_EXCLUDE and _SHALLOW_COPY_FIELDS is shallow-copied (controlled by _SHALLOW_COPY_FIELDS), while a field in DEFAULT_EXCLUDE only is still deep-copied. This addresses review comment #11 ensuring the two class variables control independent concerns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

moonbox3 added 5 commits February 2, 2026 13:23

AI triage bot initial commit for testing

39e6bff

Update triage yaml

1a63e76

Add docs bot yaml

58ac724

Merge branch 'main' into ai-triage-bot

ece7a14

moonbox3 commented Feb 18, 2026

View reviewed changes

moonbox3 closed this Feb 18, 2026


		executor = FutureExecutor(id="future")

		assert dict[str, Any] in executor._handlers

Conversation

moonbox3 commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

moonbox3 left a comment

Choose a reason for hiding this comment

Automated PR Review Round 1: CHANGES_REQUESTED

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

moonbox3 left a comment

Choose a reason for hiding this comment

Automated PR Review Round 2: CHANGES_REQUESTED

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

moonbox3 commented Feb 18, 2026 •

edited

Loading