Skills: retarget DevFlow skills to the unified maui CLI#198
Conversation
Code review (3 models) — findings + fixesRan the changes through 3 parallel CRITICAL — fixed
HIGH — fixed
Deferred (out of scope, follow-up)
Verification after fix
Reviewers' raw output is preserved in the session log if anyone wants to dig in. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Expert Code Review: 4 findings posted inline (2 critical, 2 moderate). See the lean summary comment for methodology and details.
Generated by Expert Code Review (auto) for issue #198 · ● 45.9M
0bdb96f to
824dba6
Compare
There was a problem hiding this comment.
Pull request overview
Retargets the DevFlow agent skill documentation under plugins/dotnet-maui/skills/ to prefer the unified maui CLI for device setup/management, and expands guidance around --json output and structured error handling so agents can branch on machine-readable failures.
Changes:
- Update
maui-devflow-debug/maui-devflow-onboardto lead withmauiCLI commands (doctor/device/android/apple/devflow). - Rewrite Android and Apple reference docs to be
maui-first, moving raw-tool usage into explicitly labeled fallback sections. - Add/expand JSON and error-envelope guidance (codes, remediation,
--ci) across skills and troubleshooting references.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/dotnet-maui/skills/maui-devflow-onboard/SKILL.md | Adds maui doctor and JSON/error-handling guidance to onboarding verification steps. |
| plugins/dotnet-maui/skills/maui-devflow-debug/SKILL.md | Updates INVOKES to prefer unified maui CLI and adds a “Reading machine-readable output” section. |
| plugins/dotnet-maui/skills/maui-devflow-debug/references/android.md | Rewrites Android workflows to use maui android/maui device first; keeps adb as labeled fallbacks. |
| plugins/dotnet-maui/skills/maui-devflow-debug/references/ios-and-mac.md | Reorients Apple guidance around maui apple ... discovery/lifecycle and adds fallback sections for raw simctl. |
| plugins/dotnet-maui/skills/maui-devflow-debug/references/connectivity.md | Leads with maui devflow diagnose and adds a CLI error-reading section. |
| plugins/dotnet-maui/skills/maui-devflow-debug/references/troubleshooting.md | Adds a code taxonomy/remediation explanation and updates troubleshooting steps to reference maui commands/codes. |
| plugins/dotnet-maui/skills/maui-devflow-debug/references/setup.md | Makes Microsoft.Maui.Cli the only required global tool; moves other tools to optional extras and adds verification steps. |
Expert Code Review — PR #198Methodology: 3 independent reviewers with adversarial consensus 6 findings posted as inline comments (2 🔴 critical, 3 🟡 moderate, 1 🟢 minor) Findings Summary
Discarded Findings
CI StatusAll checks passing (license/cla ✅, Agent ✅, Upload results ✅). The current Test CoverageThis PR contains only markdown skill documentation — no code changes requiring tests. Validation is by content accuracy review (which this review provides) and grep verification as noted in the PR description.
|
There was a problem hiding this comment.
Expert Code Review: 6 findings posted inline (2 critical, 3 moderate, 1 minor omitted from inline). See the summary comment for full details.
Generated by Expert Code Review (auto) for issue #198 · ● 35.6M
Rewrite the `maui-devflow-debug` and `maui-devflow-onboard` skills so AI agents prefer the unified `maui` CLI for device prep and management instead of mixing `androidsdk.tool`, `appledev.tools`, raw `adb`, and raw `xcrun simctl`. Raw fallbacks are kept only for operations the `maui` CLI does not yet wrap, and are grouped under clearly labeled sections. Also adds inline JSON / error troubleshooting guidance so agents pass `--json`, branch on `error.code` (E1xxx tool, E2xxx platform/SDK, E3xxx user, E4xxx network, E5xxx permission), and follow `error.remediation` hints emitted by `MauiToolException`. - maui-devflow-debug/SKILL.md: INVOKES rewritten; new "Reading machine- readable output" section. - references/android.md: full rewrite, `maui android …` first; raw `adb` ops grouped as fallbacks. - references/ios-and-mac.md: `maui apple xcode|runtime|simulator …` first; raw `xcrun simctl` (create/erase/install/launch/privacy/ appearance/openurl/push/location/addmedia) grouped as fallbacks. - references/connectivity.md: `maui devflow diagnose` + `maui device list` first; new "Reading errors from the CLI" section. - references/troubleshooting.md: new "Reading machine-readable output and errors" top section with full envelope, code taxonomy, worked example (E2106). - references/setup.md: `Microsoft.Maui.Cli` is the only required global tool; optional extras called out; raw `adb` port forwarding labeled; `maui doctor` added to verify. - maui-devflow-onboard/SKILL.md: `maui doctor` + `--json`/error.code guidance added to verify step. Closes #197 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three-model code review (Sonnet 4.6, Opus 4.7, GPT-5.4) caught real factual bugs in the previous commit. All converged on the same critical issues: 1. **JSON envelope shape was wrong.** `JsonOutputFormatter.WriteError` serializes `ErrorResult` directly, with no `"error"` wrapper. Property names are `snake_case` (`native_error`, `manual_steps`, `docs_url`), not camelCase. Fixed all four files that documented the wrong shape: troubleshooting.md, connectivity.md, SKILL.md, onboard SKILL.md. 2. **`remediation.type` is lowercase on the wire** (`autofixable`, `useraction`, `terminal`, `unknown`) — see ErrorResult.cs:81 `.ToString().ToLowerInvariant()`. Skills documented PascalCase, which would never match. Fixed everywhere. 3. **`maui android sdk install|uninstall` takes packages positionally**, not via `--package`. Replaced six call sites in android.md plus one in troubleshooting.md. 4. **Removed fictional commands**: `maui android sdk info`, `maui android sdk download`, `maui android jdk info`, `maui android sdk list --installed`. None are registered. 5. **Worked example (E2106) was misleading.** The `--name` throw site doesn't emit a `remediation` block; only the AvdManager "emulator binary not installed" path does, and its command is `maui android sdk install emulator` (positional). Replaced the example and added explicit guidance: when `remediation` is absent from the envelope, surface `message` and stop retrying. 6. **Promoted `maui devflow ui permission grant|revoke|reset` from raw fallback to primary path** in ios-and-mac.md. The wrapper exists (DevFlowCommands.cs:697-741) and auto-detects UDID + bundle id; the skill was sending agents past it to raw `xcrun simctl privacy`. Also removed `privacy` from the SKILL.md fallback list and added a cross-reference from the simctl utilities table. Verified all `maui …` snippets remaining in the skills against `src/Cli/Microsoft.Maui.Cli/Commands/` and `Models/ErrorResult.cs`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace 'maui devflow ui logs' (does not exist) with 'maui devflow logs' in android.md and troubleshooting.md - Fix E2106 example in connectivity.md — the 'no AVD with that name' throw site does NOT emit autofixable remediation; only the 'emulator binary not installed' site does. Use the documented site and add note about the no-remediation variant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1306774 to
bbc8e1b
Compare
|
/review |
|
✅ Expert Code Review completed successfully! |
Expert Code Review — PR #198Methodology: 3 independent reviewers with adversarial consensus 5 findings posted as inline comments (3 🔴 critical, 2 🟡 moderate) Findings Summary
Discarded Findings (single reviewer only — no consensus)
CI StatusAll checks passing: ✅ license/cla, ✅ check, ✅ pr-status (evaluate/gate skipped — expected for docs-only PR) Test CoverageNo tests needed — this PR modifies only markdown skill documentation files. Validation is by content review and
Warning
|
There was a problem hiding this comment.
Expert Code Review: 5 findings posted inline (3 critical, 2 moderate). See the lean summary comment for full details.
Warning
⚠️ Firewall blocked 3 domains
The following domains were blocked by the firewall during workflow execution:
pypi.orgspec.commonmark.orgtalk.commonmark.org
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "pypi.org"
- "spec.commonmark.org"
- "talk.commonmark.org"See Network Configuration for more information.
Generated by Expert Code Review for issue #198 · ● 34.3M
| maui devflow ui tree --depth 1 | ||
| ``` | ||
|
|
||
| For programmatic use, pass `--json` to `maui doctor` / `maui devflow |
There was a problem hiding this comment.
🔴 CRITICAL · 3/3 consensus (validated via dispute follow-up)
maui doctor --json does not output a flat code/remediation.command envelope. It outputs a DoctorReport schema:
{ "schema_version": "1.0", "status": "Unhealthy", "checks": [{ "name": "...", "status": "Error", "fix": { "auto_fixable": true, "command": "..." } }], "summary": {...} }Agents following this guidance will find no code/remediation fields in normal output and may incorrectly declare the environment healthy.
Recommendation: For maui doctor --json, instruct agents to check top-level status (Healthy/Unhealthy/Degraded) and iterate checks[] for entries with status == "Error", acting on fix.command when fix.auto_fixable == true. Reserve code/remediation parsing for maui android *, maui apple *, and other non-doctor commands.
| Prefer the unified `maui` CLI for simulator/runtime/Xcode discovery and basic | ||
| lifecycle. Permission management uses `maui devflow ui permission` (wraps | ||
| `xcrun simctl privacy` with auto-detection). Raw `xcrun simctl` is kept only | ||
| for operations not yet wrapped by the `maui` CLI (create / erase / install / |
There was a problem hiding this comment.
🔴 CRITICAL · 3/3 consensus
The CLI source (AppleCommands.cs) registers maui apple simulator create, erase, install, uninstall, launch, and terminate — these are wrapped. Listing them as "not yet wrapped" causes agents to use raw xcrun simctl instead of the unified CLI, losing --json output, structured errors, and --if-not-exists idempotency on create.
Recommendation: Move create, erase, install, uninstall, launch, and terminate to the "maui CLI" sections with correct syntax. Keep only appearance, openurl, push, location, addmedia, and io screenshot as true raw fallbacks.
| error NETSDK1147: To build this project, the following workloads must be installed: maui-ios | ||
| ``` | ||
| Fix: `dotnet workload install maui` (installs all MAUI workloads). | ||
| Error code via `maui` JSON: `E2402` MauiWorkloadMissing (often `AutoFixable`). |
There was a problem hiding this comment.
🟡 MODERATE · 2/3 consensus
`AutoFixable` (PascalCase in backticks) contradicts the rest of this document which specifies that remediation.type is always lowercase (serialized via .ToString().ToLowerInvariant()). An agent comparing remediation.type == "AutoFixable" will miss the auto-fix opportunity.
Recommendation: Change to (often `autofixable`).
| ### Error envelope | ||
|
|
||
| **Quick reference** — when a non-DevFlow command fails with `--json`, stdout is a top-level JSON object (no `"error"` wrapper). Note: `maui devflow ...` uses a different JSON error shape and writes errors to stderr. | ||
| When a `maui` command fails with `--json`, it writes a structured error object |
There was a problem hiding this comment.
🔴 CRITICAL · 3/3 consensus
The old text had an important caveat: "Note: maui devflow ... uses a different JSON error shape and writes errors to stderr." This was removed. Without it, agents will try to parse maui devflow diagnose --json output using the flat stdout code/remediation envelope described here, but DevFlow commands write {"error":"...","type":"...","retryable":...} to stderr.
Recommendation: Restore the caveat, e.g.: "Note: maui devflow ... commands emit errors to stderr in a different JSON shape ({error, type, retryable}). The flat error envelope described below applies only to non-DevFlow maui commands (maui android ..., maui apple ..., maui doctor, maui device ...)."
|
|
||
| Available services: `all`, `calendar`, `contacts`, `contacts-limited`, `location`, `location-always`, `photos`, `photos-add`, `media-library`, `microphone`, `motion`, `reminders`, `siri`. | ||
| `--udid <UDID>` is optional and only needed if multiple simulators are | ||
| booted; `--bundle-id` may be omitted when the DevFlow agent has it cached. |
There was a problem hiding this comment.
🟡 MODERATE · 2/3 consensus (validated via dispute follow-up)
The claim "--bundle-id may be omitted when the DevFlow agent has it cached" is incorrect. The implementation (DevFlowCommands.cs) has no agent-state lookup for bundle ID — when omitted, it simply invokes xcrun simctl privacy <UDID> <action> <service> without a bundle identifier. Apple's simctl privacy requires a bundle ID for grant and revoke (only reset all works without one), so omitting it will cause silent failures.
Recommendation: Document --bundle-id as required for grant/revoke operations. Only mark it as optional for reset all.
|
/evaluate |
Skill Evaluation Results20260603-171744No markdown report |
- setup.md: add 'Guided environment setup' (maui android install, maui apple install --platform, --sdk/--jdk overrides); trim 'Optional extras' now that CLI wraps create/erase/install/launch/appearance. - ios-and-mac.md: replace raw 'xcrun simctl install/launch/uninstall/get_app_container' block with 'maui apple simulator' wrappers; rewrite Dark Mode Testing to use 'maui devflow theme set/get'. - connectivity.md: document 'maui devflow extensions list|describe|call' for app-registered extension tools. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Retargets the
maui-devflow-debugandmaui-devflow-onboardagent skills (underplugins/dotnet-maui/skills/) so AI agents prefer the unifiedmauiCLI (Microsoft.Maui.Cli) for device prep and management, instead of mixingandroidsdk.tool(android …),appledev.tools(apple …), rawadb, and rawxcrun simctl.Raw fallbacks are kept only for operations the
mauiCLI does not yet wrap, and are grouped under clearly labeled sections so future CLI work has a concrete target list.Also adds inline JSON / error-troubleshooting guidance so agents pass
--json, branch on error codes, and follow remediation hints emitted byMauiToolException.Closes #197
What's in this PR (skill markdown only)
maui-devflow-debug/SKILL.md— INVOKES rewritten to lead withmauiCLI; new "Reading machine-readable output" section covering--json, error code categoriesE1xxx–E5xxx,remediation.type(autofixable/useraction/terminal— lowercase), and--ci. Explicitly notes thatmaui devflowcommands use a different stderr JSON shape.references/android.md— fully rewrittenmaui-first. Rawadbops (port forwarding, install, logcat, screencap, push/pull/shell) grouped under "Raw fallbacks not yet inmauiCLI".sdk installuses positional args (no--packageflag).references/ios-and-mac.md— restructured aroundmaui apple xcode|runtime|simulator …including newly promoted commands:create,erase,install,uninstall,launch,terminate. Rawxcrun simctlops (appearance/openurl/push/location/addmedia/screenshots) grouped under the labeled fallback section. Permission usesmaui devflow ui permissionwith--bundle-iddocumented as required for per-app grants.references/connectivity.md— leads withmaui devflow diagnose+maui device list; new "Reading errors from the CLI" section scoped to non-DevFlow commands with the flat JSON error envelope (no"error"wrapper).references/troubleshooting.md— new top section "Machine-readable output and error envelope" with flat JSON (no wrapper), code taxonomy, lowercase remediation types,manual_steps(snake_case), and a worked example (E2106emulator binary missing →maui android sdk install emulator).references/setup.md—Microsoft.Maui.Cliis the only required global tool; optional extras shrunk to only truly unwrapped ops;maui doctoradded to verify.maui-devflow-onboard/SKILL.md—maui doctor+--jsonerror guidance added to verify step, scoped to non-DevFlow commands with DevFlow stderr shape noted.references/macos.md,linux.md, andbatch.mdwere verified clean (no changes needed).Audit highlights
Verified against
src/Cli/Microsoft.Maui.Cli/Commands/andModels/ErrorResult.cs.android avd|sdk|jdk|device …→maui android emulator|sdk|jdk …adb reverse|forward(broker port + agent port)apple simulator list/start/stop/delete/create/erase/install/launch/terminate→maui apple simulator …adb install|uninstall,am start|force-stop,pm uninstalladb devices/xcrun simctl list devices→maui device list --platform …adb logcat,adb shell screencap|screenrecord,adb push|pull|shellmaui devflow diagnosefirstxcrun simctl appearance|openurl|push|location|addmediamaui doctoradded to setup + onboard verifylsof -i :<port>,pgrep -f "App"JSON / error troubleshooting improvements
Skills now teach agents to:
--jsonto any command that will be parsed.codefield (flat JSON, no"error"wrapper):E1xxx— tool/internalE2xxx— platform/SDK (E20xxJDK,E21xxAndroid,E22xxApple,E23xxWindows,E24xx.NET)E3xxx— user actionE4xxx— networkE5xxx— permissionremediation.type == "autofixable"(lowercase), runremediation.commandand retry.remediation.type == "useraction", followremediation.manual_steps(snake_case).--cifor non-interactive failure-fast runs.maui devflow …) use a different error shape ({"error":"…","type":"…","retryable":…}) written to stderr — not theErrorResultenvelope.A worked example (auto-fixable
E2106recovery: missing emulator binary →maui android sdk install emulator) is included inreferences/troubleshooting.md.Verification
Every remaining match is inside a clearly labeled "Raw fallbacks not yet in
mauiCLI" section or is preceded by a# Not yet wrapped by 'maui' CLIcomment.Post-review fixes verified with multi-model code review (GPT-5.4, Claude Opus 4.7, Claude Haiku 4.5) and all 19 review threads resolved.
Out of scope
maui-ai-debuggingskill (already redirects tomaui-devflow-debug).devflow-automationskill (about[DevFlowAction], not devices).mauiCLI commands — the follow-up CLI gaps are tracked in DevFlow skills: prefer unified maui CLI over standalone tools + improve JSON/error guidance #197 and intentionally deferred.Test plan
These are markdown skill files only — no build/test target. Validation is by re-read + grep verification (above), automated code review (2 rounds), and multi-model review.