[skills] Improve macios-binding-creator skill #25906
Conversation
Enhance the macios-binding-creator agent skill with binding patterns and test workflows learned across several Apple-framework binding sessions, and address the review feedback from #25133: - Availability version determination via the generated reference bindings (tests/xtro-sharpie/api/), including per-member enum availability. - ObjC type-name casing: the class prefix is preserved, but acronyms inside the name follow .NET rules (NSURLSession -> NSUrlSession). - Platform conditionals: prefer __MACOS__/__TVOS__ over MONOMAC/TVOS. - NativeObject marshal-type registration (CORE_SOURCES, TypeCache, MarshalTypeList) with the correct #if !COREBUILD shell placement. - C callback handler patterns (BlockLiteral trampoline, GCHandle). - Runtime-only protocol conformance -> test-only introspection Skip. - Shared AppKit/UIKit type consolidation into src/xkit.cs. - Struct binding rules: no arrays and no explicit layout. - Per-platform monotouch-test / introspection / xtro commands. Adds a training log documenting the rationale behind each change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merge the stashed macios-binding-creator skill work (Xcode 27 xtro/cecil/introspection accuracy from three real binding sessions) onto the rebased branch, which had already committed two other 2026-07-01 sessions (xkit.cs consolidation + runtime-only protocol conformance). 3-way merge against the clean common base preserves all three sessions with no content lost either way. Also fixes two claims in the session-3 content that went stale after the branch's main-merge pulled in their source PRs (#25811, #25828): - AUHeadTrackingBinauralRenderer is now a real factory-variant type; the illustrative plain-Constructor example is renamed to MySpatialAudioUnit and the real type is cited as a factory precedent. - iOSApiSelectorTest.cs already has a Skip(Type,string) override, so the selector-skip guidance now says to extend it (adding a second override would be CS0111), citing the AVAssetWriter precedent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the macios-binding-creator agent skill documentation to better reflect the current dotnet/macios binding workflow (availability sourcing, platform-conditional patterns, and validation via xtro/cecil/introspection/monotouch tests).
Changes:
- Clarifies that availability versions should come from xtro-generated reference bindings / SDK headers (not blindly from current SDK versions), including per-enum-member availability guidance.
- Corrects and expands test workflow guidance (xtro
gen-all+dotnet-classify, sanity cleanup loop, cecil doc-baseline regeneration, selector/protocol-conformance troubleshooting). - Adds/expands binding patterns guidance (naming prefix casing, platform directives usage, NativeObject marshal types for protocol returns, designated initializer re-exposure, shared xkit binding notes).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .agents/skills/macios-binding-creator/SKILL.md | Updates core skill steps for version sourcing, platform guards, build commands, and test workflows. |
| .agents/skills/macios-binding-creator/references/test-workflow.md | Expands concrete commands and troubleshooting for xtro/cecil/introspection/monotouch-test runs. |
| .agents/skills/macios-binding-creator/references/binding-patterns.md | Adds detailed binding patterns (enum member availability, protocol conformance handling, marshal type registration, etc.). |
| .agents/files/training-log-macios-binding-creator.md | Adds a training log capturing rationale and provenance for the skill updates. |
| make all && make install | ||
| ``` | ||
|
|
||
| > ❌ **NEVER** use `make -C src build`. There is no `build` target in `src/Makefile`, so it matches the `src/build/` output directory and is a silent no-op ("Nothing to be done for `build'") that compiles nothing — you then validate against **stale** assemblies. Use `make all && make install` (or `make world` for a full rebuild). |
|
|
||
| > ⚠️ **Platform casing matters**: Use `iOS`, `tvOS`, `macOS`, `MacCatalyst` exactly — not `ios`, `macos`, etc. | ||
|
|
||
| > ⚠️ **Desktop platforms**: Use `run-bare` (not `run`) for macOS and MacCatalyst — same reason as introspection: `run` launches without capturing stdout. `run-bare` is only available for desktop platforms. |
|
|
||
| > ⚠️ **Desktop platforms (macOS, MacCatalyst)**: Use `run-bare` for captured test output — same as introspection. `run` launches the app via `dotnet build -t:Run` which doesn't capture stdout. | ||
|
|
||
| > ⚠️ **`run-bare` is desktop-only.** iOS and tvOS use the simulator via `dotnet build -t:Run` with `SIMCTL_CHILD_NUNIT_AUTOSTART=true` and `SIMCTL_CHILD_NUNIT_AUTOEXIT=true` environment variables (set automatically by the shared Makefile). No manual mlaunch invocation is needed for monotouch-tests — unlike introspection. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rolfbjarne
left a comment
rolfbjarne
left a comment
There was a problem hiding this comment.
Looking good, just a few small nitpicks!
Fix the 9 review comments on the macios-binding-creator skill (6 from rolfbjarne, 3 from the Copilot reviewer), each verified against the current tree: - Callback trampolines: the function-pointer target must be a named static method with [UnmanagedCallersOnly] (a lambda can't carry it). - xkit.cs consolidation also applies in reverse (uikit.cs -> AppKit). - macOS/UIKit divergences: prefer [No*] attributes over #if __MACOS__; reserve #if for divergences attributes can't express (per-platform Export/semantic/nullability, differing name, differing protocol list). - Re-exposing designated initializers: a failable init (out NSError) must use the [Internal] _Init + Create factory, because ConstructorTest forbids constructors with an out NSError parameter. - Naming: also match the casing of existing sibling APIs on a type. - Note AUTO_SANITIZE=1 for auto-removing resolved xtro .todo/.ignore lines. - Clarify 'make -C src build' (no build target) and that run-bare exists on every platform but can't launch iOS/tvOS. Also records a training-log entry for this feedback round. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #f30ca74] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [PR Build #f30ca74] Build passed (Build macOS tests) ✅Pipeline on Agent |
✅ [PR Build #f30ca74] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #f30ca74] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 196 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
No description provided.