feat: Add reflection invoke for DevFlow Actions and arbitrary methods#149
Conversation
There was a problem hiding this comment.
Expert Code Review — PR #149: Reflection Invoke for DevFlow Actions
Methodology: 3 independent reviewers with adversarial consensus. Disputed findings (flagged by only 1 reviewer) were verified by the other 2 reviewers before inclusion or discard. No findings were discarded.
Summary: 12 findings (2 critical, 7 moderate, 3 minor)
| # | Severity | Consensus | Finding |
|---|---|---|---|
| 1 | 🔴 CRITICAL | 3/3 reviewers | Async element methods double-invoked — HandleElementInvoke calls method.Invoke() to detect Task, abandons the returned Task, then re-invokes the method. Side effects execute twice. |
| 2 | 🔴 CRITICAL | 3/3 reviewers | Open reflection invoke resolves framework types — ResolveType lacks the IsFrameworkAssembly filter, allowing invocation of System.IO.File, System.Diagnostics.Process, etc. |
| 3 | 🟡 MODERATE | 3/3 reviewers | _typeResolutionCache not thread-safe — Plain Dictionary with concurrent read/write from HTTP handlers. Use ConcurrentDictionary. |
| 4 | 🟡 MODERATE | 3/3 reviewers | ValueTask not awaited; analyzer falsely approves — Runtime only checks is Task; ValueTask (a struct) never matches. Analyzer's IsWellKnownReturnType accepts both. |
| 5 | 🟡 MODERATE | 3/3 reviewers | ICollection<T>/IReadOnlyCollection<T> analyzer-runtime mismatch — Analyzer accepts them, runtime throws ArgumentException. |
| 6 | 🟡 MODERATE | 2/3 reviewers | IsRequired nullable check is a no-op — IsAssignableTo(typeof(Nullable<>)) always returns false for open generic. |
| 7 | 🟡 MODERATE | 2/3 reviewers | Duplicate action names silently shadow — No detection at runtime; PR description incorrectly claims DFA004 covers this. |
| 8 | 🟡 MODERATE | 2/3 reviewers | _cachedActions race condition — Non-volatile field with unsynchronized double-check pattern. |
| 9 | 🟡 MODERATE | 2/3 reviewers | GetMethod throws AmbiguousMatchException — Overloaded methods (e.g., ToString) hit this before fallback logic runs. |
| 10 | 🟢 MINOR | 2/3 after follow-up | bool.Parse(null!) NRE path — Non-string JSON kinds (e.g., numeric) fall through to GetString() → null. |
| 11 | 🟢 MINOR | 3/3 after follow-up | Test coverage gaps — No tests for element invoke, DI service resolution, array/enum/nullable params. |
| 12 | 🟢 MINOR | 3/3 after follow-up | Action cache never refreshed — Late-loaded assemblies invisible to maui_list_actions but resolvable by maui_invoke. |
CI Status
- ✅
license/cla— passed - ✅
build / build (ubuntu-24.04)— passed - 🔄
build / build (macos-latest)— in progress - 🔄
build / build (windows-latest)— in progress
Test Coverage Assessment
11 new tests cover the happy paths well (action discovery, static invoke, async/void methods, bool params, error cases). However, the more complex and bug-prone paths — element invoke, DI service resolution, array/list conversion, enum conversion, and nullable handling — have no test coverage. The two critical bugs (double invocation, ValueTask handling) both live in untested code paths.
Generated by Expert Code Review (auto) for issue #149 · ● 18.1M
|
@vitek-karas this is what I had in mind when you brought up the idea of having devflow be able to invoke methods in the running app via the agent. There's two routes, one is simply, reflection based, the other is making the concept more of a feature and being able to articulate which methods and their parameters are available and what for to the CLI. |
There was a problem hiding this comment.
Pull request overview
Adds a reflection-based “invoke” capability to DevFlow so AI agents (via CLI/MCP) can quickly set app state by calling in-app helpers directly, plus a Roslyn analyzer and test coverage to guide safe usage.
Changes:
- Introduces new Agent.Core invoke endpoints + action discovery (
[DevFlowAction]) with caching and hot-reload invalidation. - Extends the public Driver (
AgentClient) with invoke/list APIs andInvokeResult, and exposes new MCP tools (maui_list_actions,maui_invoke_action,maui_invoke,maui_list_methods). - Adds a new
Microsoft.Maui.DevFlow.Analyzersproject + analyzer tests, and updates docs/skill + versioning metadata.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/DevFlow/Microsoft.Maui.DevFlow.Tests/Microsoft.Maui.DevFlow.Tests.csproj | Adds analyzer-testing package + references analyzer project for tests. |
| src/DevFlow/Microsoft.Maui.DevFlow.Tests/InvokeTests.cs | New test suite validating invoke/action/element invoke behaviors and conversions. |
| src/DevFlow/Microsoft.Maui.DevFlow.Tests/DevFlowActionAnalyzerTests.cs | New analyzer tests for DFA diagnostics. |
| src/DevFlow/Microsoft.Maui.DevFlow.Driver/DevFlowDriverJson.cs | Adds source-gen JSON registration for InvokeResult. |
| src/DevFlow/Microsoft.Maui.DevFlow.Driver/AgentClient.cs | Adds public invoke/list methods and InvokeResult DTO. |
| src/DevFlow/Microsoft.Maui.DevFlow.Analyzers/Microsoft.Maui.DevFlow.Analyzers.csproj | New analyzer project definition (netstandard2.0). |
| src/DevFlow/Microsoft.Maui.DevFlow.Analyzers/DevFlowActionAnalyzer.cs | Implements DFA001–DFA005 diagnostics for [DevFlowAction]. |
| src/DevFlow/Microsoft.Maui.DevFlow.Analyzers/AnalyzerReleases.Unshipped.md | Tracks unshipped analyzer rules. |
| src/DevFlow/Microsoft.Maui.DevFlow.Analyzers/AnalyzerReleases.Shipped.md | Tracks shipped analyzer rules. |
| src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/Microsoft.Maui.DevFlow.Agent.Core.csproj | Wires analyzer into Agent.Core via OutputItemType="Analyzer". |
| src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/DevFlowAgentService.cs | Makes service partial; registers new invoke routes; advertises invoke capabilities; adds batch invoke support and AssemblyLoad hook. |
| src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/DevFlowAgentService.Invoke.cs | New partial implementing discovery/type resolution/arg conversion/invoke handlers + hot reload cache invalidation. |
| src/DevFlow/Microsoft.Maui.DevFlow.Agent.Core/DevFlowActionAttribute.cs | New attribute for registering discoverable actions with parameter descriptions. |
| src/DevFlow/DevFlow.slnf | Adds analyzer project to the DevFlow solution filter. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Mcp/Tools/InvokeTools.cs | New MCP tools for action discovery/invoke and reflection invoke. |
| src/Cli/Microsoft.Maui.Cli/DevFlow/Mcp/McpServerHost.cs | Registers InvokeTools with the MCP server. |
| plugins/dotnet-maui/skills/devflow-automation/SKILL.md | New skill doc teaching agents how to use invoke/actions. |
| plugins/dotnet-maui/plugin.json | Bumps plugin version to 0.2.0. |
| eng/Versions.props | Pins Microsoft.CodeAnalysis.CSharp version. |
| MauiLabs.slnx | Adds analyzer project to the main solution. |
| Directory.Packages.props | Adds central package versions for Roslyn dependencies and analyzer testing. |
There was a problem hiding this comment.
Expert Code Review — PR #149: Reflection Invoke for DevFlow Actions
Methodology: 3 independent reviewers with adversarial consensus. Disputed findings (flagged by only 1 reviewer) were verified by the other 2 reviewers before inclusion or discard.
Summary: 8 findings (1 critical, 5 moderate, 2 minor)
| # | Severity | Consensus | File | Line(s) | Finding |
|---|---|---|---|---|---|
| 1 | 🔴 CRITICAL | 2/3 reviewers | DevFlowAgentService.Invoke.cs |
456, 545 | HandleInvokeAction and static HandleInvoke not dispatched to UI thread — InvokeMethodAsync runs on the HTTP server's background thread. The PR's canonical example (Shell.Current.GoToAsync) requires the UI thread and will crash on iOS/Android/Windows. |
| 2 | 🟡 MODERATE | 2/3 reviewers | DevFlowAgentService.Invoke.cs |
110–121 | IsFrameworkAssembly missing Microsoft.Maui.* and other prefixes — Filter doesn't block MAUI framework types, DevFlow's own types, Microsoft.CSharp, or Microsoft.Win32. |
| 3 | 🟡 MODERATE | 3/3 after follow-up | DevFlowAgentService.Invoke.cs |
413–414 | Invoke errors return HTTP 200, breaking batch success tracking — InvokeError returns 200; batch handler checks StatusCode < 400, so failed invokes count as succeeded. |
| 4 | 🟡 MODERATE | 3/3 after follow-up | DevFlowAgentService.Invoke.cs |
575–576 | HandleElementInvoke has no framework-assembly check — Any public instance method on MAUI framework types is invokable, bypassing ResolveType's filter. |
| 5 | 🟡 MODERATE | 2/3 reviewers | DevFlowAgentService.Invoke.cs |
501–507 | Overload resolution picks by argument count only — Same-arity overloads with different parameter types silently pick the first in metadata order. |
| 6 | 🟡 MODERATE | 2/3 after follow-up | DevFlowAgentService.Invoke.cs |
539–541 | Service invoke async continuations may run off UI thread — Value-type tuple causes Func<T> overload resolution instead of Func<Task<T?>>, so only the start of InvokeMethodAsync runs on the UI thread. |
| 7 | 🟢 MINOR | 2/3 reviewers | DevFlowAgentService.Invoke.cs |
210–218 | ResolveType simple-name fallback non-deterministic — Last match wins when multiple assemblies contain a type with the same simple name. |
| 8 | 🟢 MINOR | 2/3 reviewers | DevFlowAgentService.Invoke.cs |
43–46 | _typeResolutionCache not invalidated on Hot Reload — InvalidateActionCache() (called by Hot Reload handler) doesn't clear the type resolution cache; scope mismatch between static action cache and instance type cache. |
Discarded findings (single reviewer only, not confirmed by follow-up)
10 findings were flagged by only 1 of 3 reviewers and discarded per the adversarial consensus protocol (cap of 3 follow-ups reached): HandleElementInvoke uses GetMethod() vs GetMethods(), HandleListMethods HTTP status inconsistency, broad exception handling gaps, analyzer namespace matching, URL encoding of elementId, exception message leakage, analyzer ConcurrentBag ordering, FormatParameterTypeName gaps, negative type lookup caching, and analyzer diagnostic ID documentation mismatch.
CI Status
| Check | Status |
|---|---|
license/cla |
✅ Passed |
build / build (ubuntu-24.04) |
✅ Passed |
build / build (macos-latest) |
✅ Passed (ci-devflow), ❌ Failed (ci-cli) |
build / build (windows-latest) |
🔄 In progress |
Test Coverage Assessment
44 new tests (33 invoke + 11 analyzer) cover the happy paths well — action discovery, static invoke, async/void methods, parameter conversion, and error handling. The critical UI-thread-dispatch bug (finding #1) and the batch integration issue (finding #3) are both in untested code paths. Element invoke, DI service resolution, and the overload resolution edge cases also lack coverage.
Generated by Expert Code Review · 3 independent reviewers with adversarial consensus
Generated by Expert Code Review (auto) for issue #149 · ● 25.2M
|
@Redth I like this attribute approach - definitely a good start. Fully random reflection is a bit too wild. And the attributes would allow for "no-reflection" (with source generators). |
52ce14b to
352d230
Compare
Two-tier invoke system for AI agents to rapidly set up app state: Tier 1 — DevFlow Actions: Methods annotated with [DevFlowAction] are discoverable via maui_list_actions. Rich parameter metadata via [Description] attributes, just like MCP tools. Validated at compile time by a new Roslyn analyzer (MAUI_DFA001-004). Tier 2 — Open reflection invoke: Call any public static method or DI-resolved service method by type+method name. Enables AI to call helpers it discovers from source code, even without [DevFlowAction]. Changes across all DevFlow layers: - Agent.Core: 5 HTTP endpoints, assembly scanning, type resolution, parameter conversion (primitives, enums, arrays/lists, nullable), async method unwrapping (Task/Task<T>), batch integration - Driver: 5 AgentClient methods + InvokeResult DTO + source-gen registration - CLI/MCP: 4 tools (maui_list_actions, maui_invoke_action, maui_invoke, maui_list_methods) with rich [Description] text - Analyzer: New Microsoft.Maui.DevFlow.Analyzers project (netstandard2.0) with 4 diagnostics for compile-time validation of [DevFlowAction] methods - Skill: New devflow-automation skill teaching AI agents when and how to use invoke as a first-class automation technique - Plugin version bumped 0.1.0 → 0.2.0 Tests: 11 new tests covering action discovery, invoke by name, invoke by type+method, async methods, void methods, bool params, error cases. All 158 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…code execution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Subscribe to AppDomain.AssemblyLoad to reset the cached actions when new non-framework assemblies are loaded (Blazor Hybrid lazy loading, plugin architectures). Add MetadataUpdateHandler so C# Hot Reload / Edit and Continue updates also invalidate the cache -- newly added [DevFlowAction] methods become immediately discoverable without restarting the app. The cache field is now static volatile Lazy<T> (resettable) instead of readonly Lazy<T>. Type resolution cache is also cleared on new assembly load since it may contain stale miss entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ests - Add MAUI_DFA005 (Warning): detects duplicate [DevFlowAction] names across the compilation using CompilationStart/End pattern - Refactor analyzer to use RegisterCompilationStartAction for duplicate tracking - Add Microsoft.CodeAnalysis.CSharp.Analyzer.Testing package - Create DevFlowActionAnalyzerTests with 11 tests covering DFA001-DFA005 - Update AnalyzerReleases.Unshipped.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 10 new tests that exercise the AgentClient invoke methods using the same parameter patterns that MCP InvokeTools pass: - JSON string parsing (argsJson → JsonArray) for InvokeAction and Invoke - Explicit resolve: "static" parameter - resolve: "service" error when no DI container is available - Overloaded method resolution by argument count - Mixed-type and nested array argument conversion Also adds TestInvokeHelpersWithOverloads (2-arg and 3-arg Concat) and TestServiceClass (instance methods) as test fixture types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… framework filtering, type resolution ambiguity, overload selection, documentation fixes - ConvertInvokeArg: check ValueKind before GetString() for bool, numeric, enum paths - Enum conversion: handle Number via Enum.ToObject, String via Enum.Parse - HandleInvokeAction/HandleInvoke: catch Exception instead of only ArgumentException - IsFrameworkAssembly: use TRUSTED_PLATFORM_ASSEMBLIES for comprehensive filtering - ResolveType: detect ambiguous simple-name matches across assemblies - Overload selection: explicit error when multiple overloads match arg count - ListMethods: detect ValueTask/ValueTask<T> as async - MCP tool description: remove misleading element reference mention - Analyzer tests: remove redundant DescriptionAttribute stub - AnalyzerReleases: move all rules to Unshipped (no release yet) - SKILL.md: add DFA005 diagnostic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Select invoke overloads using conservative JSON argument convertibility so obvious string and numeric overloads resolve without depending on metadata order, while equally compatible overloads still return an ambiguity error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove open reflection invoke, method discovery, element invoke, and generic invoke batch support so DevFlow automation only calls methods explicitly annotated with DevFlowAction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fd9efc0 to
06139d3
Compare
Prefer app/default-context DevFlow actions when duplicate action names are discovered so stale or secondary assembly loads do not shadow the running app action. Pin Roslyn CSharp.Workspaces with the repo Roslyn version to avoid NU1701 restore warnings from analyzer test dependencies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve the dotnet-maui plugin manifest conflict by preserving the PR version bump and the updated main branch description. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Motivation
AI agents automating MAUI apps currently interact one step at a time -- tap, screenshot, read tree, repeat. For common setup tasks like logging in a test user, seeding data, or navigating to a deeply nested screen, this round-trip loop is slow.
This PR adds explicitly registered DevFlow Actions: app-defined shortcuts that AI agents can discover and invoke in the running app.
Approach
App developers opt methods into automation with
[DevFlowAction]:Actions are discoverable via
maui_list_actionsand invokable viamaui_invoke_action. Parameters support[Description]attributes so AI agents get rich metadata about what to pass.This intentionally does not expose arbitrary reflection invoke. DevFlow can only call methods the app explicitly opts in with
[DevFlowAction].What changed
[DevFlowAction]attribute, action discovery endpoint (GET /api/v1/invoke/actions), action invocation endpoint (POST /api/v1/invoke/actions/{name}), assembly scanning with cache invalidation, parameter conversion (primitives, enums, arrays/lists, nullable types), async method unwrapping (Task/Task<T>/ValueTask/ValueTask<T>), batch integration (invoke-action)AgentClient.ListActionsAsync,AgentClient.InvokeActionAsync,InvokeResultDTO, source-gen JSON registrationmaui_list_actions,maui_invoke_actionMicrosoft.Maui.DevFlow.Analyzersproject (netstandard2.0) with 5 Roslyn diagnostics: unsupported param type (MAUI_DFA001), must be public static (MAUI_DFA002), return type warning (MAUI_DFA003), missing[Description](MAUI_DFA004), duplicate action name (MAUI_DFA005)devflow-automationskill doc teaching AI agents to use explicitly registered actions as a first-class technique for rapid app state manipulationSafety and thread safety
[DevFlowAction]; it does not expose type-name reflection, DI service invocation, method discovery, or element method invocation.volatile Lazy<T>withExecutionAndPublicationthread safety.DiscoverActions()detects duplicate[DevFlowAction]names at runtime (log + deduplicate), and the analyzer catches them at compile time via MAUI_DFA005.ConvertInvokeArgchecksJsonElement.ValueKindbefore converting and returns structured errors for invalid arguments.Hot Reload and dynamic assembly support
The action cache automatically invalidates in two scenarios:
AppDomain.CurrentDomain.AssemblyLoadso lazy-loaded assemblies have their[DevFlowAction]methods picked up.[MetadataUpdateHandler]resets the cache when types are modified at runtime. This means newly added[DevFlowAction]methods become discoverable without restarting the app.Non-obvious decisions
0.1.0-preview), and the retained endpoints are additive under/api/v1/invoke/actions.DevFlowAgentService.csis large, so action infrastructure lives in a separateDevFlowAgentService.Invoke.cspartial.Testing
ValueTaskreturnsinvoke-actionfailure behavior