Improve dotnet-pinvoke skill: stop signals, routing, inline anti-patterns by lewing · Pull Request #72 · dotnet/skills

lewing · 2026-02-20T20:20:48Z

Builds on PR #54

This PR stacks on #54 (Add .NET P/Invoke skill by @AaronRobinsonMSFT) — it should be reviewed/merged after #54 lands.

Changes

Improvements based on multi-model evaluation against skill-builder patterns.

What changed

Structured description keywords — Reformatted to \USE FOR:\ / \DO NOT USE FOR:\ for better routing
When to Use section — 6 concrete trigger scenarios
Stop Signals section — Prevents agents from marching through all 8 steps on simple tasks
Inline anti-patterns — 4 \❌ NEVER\ callouts near Steps 2, 4, and 5

Why

Without stop signals, an agent asked to 'write a P/Invoke for CreateFile' would go through all 8 steps including migration and tooling — now it knows to stop at Step 3 for single-function tasks.

Evidence

Multi-model eval (3 models, 2 families) scored 4.8/5 average:

Model	Score
Claude Sonnet 4	4.8/5
GPT-5.1	5.0/5
Claude Haiku 4.5	4.6/5

AaronRobinsonMSFT · 2026-02-20T22:19:23Z

plugins/dotnet/skills/dotnet-pinvoke/SKILL.md

@@ -9,6 +9,23 @@ Calling native code from .NET is powerful but unforgiving. Incorrect signatures,

 This skill covers both `DllImport` (available since .NET Framework 1.0) and `LibraryImport` (source-generated, .NET 7+). When targeting .NET Framework, always use `DllImport`. When targeting .NET 7+, prefer `LibraryImport` for new code. When native AOT is a requirement, `LibraryImport` is the only option.

+## When to Use This Skill


Ugh. I have no idea why this was removed. I think when I asked copilot to make the skill less than 500 lines it removed this. This should have been there. Apologizes @danmoseley I didn't realize you were calling out this wasn't present at all. That is on me :(

AaronRobinsonMSFT · 2026-02-20T22:20:27Z

plugins/dotnet/skills/dotnet-pinvoke/SKILL.md

+> ❌ **NEVER** use `int` or `long` for C `long` — it's 32-bit on Windows, 64-bit on Unix. Always use `CLong`.
+> ❌ **NEVER** use `ulong` for `size_t` — causes stack corruption on 32-bit. Use `nuint` or `UIntPtr`.
+> ❌ **NEVER** use `bool` without `MarshalAs` — the default marshal size is wrong.


All of these are captured in the type-mapping.md file. I don't think they should be here.

it is in the assessment https://gist.github.com/lewing/859ab56b0c37601804c03a5c601cfd8d

and the pr description. It's a pattern that works but isn't a hard rule.

I get that it is in the assessment, but what is the source of that guidance from the assessment? How do we know this is good guidance? I've been following the guidelines on anthropic and I've not seen this called out.

dotnet/runtime#124734

I get that it is in the assessment, but what is the source of that guidance from the assessment? How do we know this is good guidance? I've been following the guidelines on anthropic and I've not seen this called out.

Did you read the links I shared previously? https://github.com/lewing/agent-plugins/blob/main/plugins/skill-trainer/skills/skill-trainer-knowledge/references/skill-builder-knowledge.md#burying-critical-rules-as-numbered-workflow-steps I've found it to work multiple times in testing some of the weaker models. It's a style suggestion not a requirement. Do as you please.

dotnet/runtime#124734

AaronRobinsonMSFT · 2026-02-20T22:21:24Z

plugins/dotnet/skills/dotnet-pinvoke/SKILL.md

@@ -109,6 +130,8 @@ internal static partial int ProcessRecords(
 4. **Specify encoding explicitly.** Never rely on `CharSet.Auto`.
 5. **Never introduce `StringBuilder` for output buffers.**

+> ❌ **NEVER** rely on `CharSet.Auto` or omit string encoding — there is no safe default.


This should have been captured in the type-mapping.md file. I'm really confused on this right now.

AaronRobinsonMSFT · 2026-02-20T22:21:54Z

plugins/dotnet/skills/dotnet-pinvoke/SKILL.md

@@ -137,6 +160,8 @@ internal static partial int SetName(string name);

 When memory crosses the boundary, exactly one side must own it — and both sides must agree.

+> ❌ **NEVER** free with a mismatched allocator — `Marshal.FreeHGlobal` on `malloc`'d memory is heap corruption.


Ugh. This was another item that was in the original proposal. I'm very confused on what happend here.

danmoseley · 2026-02-21T05:24:53Z

any particular reason the tool is making a PR against main rather than either pushing to original PR or opening a PR against the original PR? presumably we want "all feedback applied" before the original PR is merged, not subsequent changes. and it would be easier to keep up :)

lewing · 2026-02-21T18:54:45Z

any particular reason the tool is making a PR against main rather than either pushing to original PR or opening a PR against the original PR? presumably we want "all feedback applied" before the original PR is merged, not subsequent changes. and it would be easier to keep up :)

I think we don't have automatic collaborators status on the forks right now?

AaronRobinsonMSFT · 2026-02-23T18:00:12Z

@adityamandaleeka I'm good with this. Feel free to merge if this passes the tooling.

Copilot

Pull request overview

Updates the .NET P/Invoke skill documentation to improve agent routing and reduce unnecessary step-by-step execution for simple interop tasks, building on the base skill added in PR #54.

Changes:

Reformats the skill frontmatter description into clearer “USE FOR / DO NOT USE FOR” routing keywords.
Adds “When to Use This Skill” triggers and a “Stop Signals” section to constrain agent behavior.
Introduces inline “❌ NEVER” anti-pattern callouts near high-risk interop guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-28T20:09:01Z

plugins/dotnet/skills/dotnet-pinvoke/SKILL.md

@@ -50,6 +75,10 @@ The most dangerous mappings — these cause the majority of bugs:

 **For the complete type mapping table, struct layout, and blittable type rules**, see [references/type-mapping.md](references/type-mapping.md).

+> ❌ **NEVER** use `int` or `long` for C `long` — it's 32-bit on Windows, 64-bit on Unix. Always use `CLong`.


The guidance about C long size is inaccurate/misleading: it’s 32-bit on Windows and typically 64-bit only on 64-bit Unix (LP64); on 32-bit Unix it’s also 32-bit. Also, saying to “always use CLong” conflicts with the earlier note that this skill covers .NET Framework (which doesn’t have CLong). Consider rewording to reflect platform/TMF constraints (e.g., use CLong/CULong when available, otherwise use conditional compilation/custom typedef wrappers).

This feedback is correct in the sense the it is implying long is always 32-bit. Which of course isn't the case and copilot is correct. The CLong type though will always do the right thing for whenever C long is used. I'm fine adding clarity here or ignoring this feedback.

Copilot · 2026-02-28T20:09:01Z

plugins/dotnet/skills/dotnet-pinvoke/SKILL.md

@@ -50,6 +75,10 @@ The most dangerous mappings — these cause the majority of bugs:

 **For the complete type mapping table, struct layout, and blittable type rules**, see [references/type-mapping.md](references/type-mapping.md).

+> ❌ **NEVER** use `int` or `long` for C `long` — it's 32-bit on Windows, 64-bit on Unix. Always use `CLong`.
+> ❌ **NEVER** use `ulong` for `size_t` — causes stack corruption on 32-bit. Use `nuint` or `UIntPtr`.
+> ❌ **NEVER** use `bool` without `MarshalAs` — the default marshal size is wrong.


“NEVER use bool without MarshalAs — the default marshal size is wrong” is too absolute and can be incorrect: the default bool marshalling matches Win32 BOOL (4 bytes), while it’s C99 _Bool/bool that requires [MarshalAs(UnmanagedType.U1)]. Suggest rephrasing to distinguish Win32 BOOL vs C99 bool, and to emphasize explicitly confirming the native boolean representation rather than banning bool outright.

Suggested change

> ❌ **NEVER** use `bool` without `MarshalAs` — the default marshal size is wrong.

> ⚠️ Do not assume the default `bool` marshalling matches native — confirm the native boolean type. For Win32 `BOOL`, the default is correct; for C99 `bool` / `_Bool`, use `[MarshalAs(UnmanagedType.U1)] bool`.

while it’s C99 _Bool/bool that requires [MarshalAs(UnmanagedType.U1)]

Minor pedantic push back. The C standard says nothing about the size of _Bool or bool. It is always 1 byte as far as I've found, but it doesn't need to be. I think the suggest feedback by copilot is fine, but the point of the NEVER was to imply the nuance the suggested comment is expressing.

jeffschwMSFT

lgtm

jeffschwMSFT · 2026-03-02T16:04:17Z

@lewing can you resolve the conflicts? after that I can merge

…nti-patterns - Reformat description to USE FOR/DO NOT USE FOR structured keywords - Add 'When to Use This Skill' section with 6 trigger scenarios - Add 'Stop Signals' section to prevent over-investigation on simple tasks - Add inline anti-patterns near Steps 2, 4, and 5 Tested with multi-model eval (Sonnet 4, GPT-5.1, Haiku 4.5) — avg 4.8/5. Assessment: https://gist.github.com/lewing/859ab56b0c37601804c03a5c601cfd8d Training log: https://gist.github.com/lewing/e272e71dca0f4d1036a02a65796b5e1c Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Single-line description with 'USE FOR:' / 'DO NOT USE FOR:' colons breaks YAML parsers that treat them as nested mappings. Switch to > block scalar. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

AaronRobinsonMSFT reviewed Feb 20, 2026

View reviewed changes

lewing closed this Feb 21, 2026

lewing deleted the improve-pinvoke-skill branch February 21, 2026 18:58

lewing restored the improve-pinvoke-skill branch February 21, 2026 19:15

lewing reopened this Feb 21, 2026

AaronRobinsonMSFT approved these changes Feb 23, 2026

View reviewed changes

lewing marked this pull request as ready for review February 28, 2026 20:05

lewing requested review from dbreshears and timheuer as code owners February 28, 2026 20:05

Copilot AI review requested due to automatic review settings February 28, 2026 20:05

Copilot started reviewing on behalf of lewing February 28, 2026 20:06 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

jeffschwMSFT approved these changes Mar 2, 2026

View reviewed changes

lewing and others added 2 commits March 2, 2026 10:45

lewing force-pushed the improve-pinvoke-skill branch from 111cc27 to 662a0d1 Compare March 2, 2026 16:46

lewing requested a review from a team as a code owner March 2, 2026 16:46

lewing merged commit 0978c88 into dotnet:main Mar 4, 2026
1 check passed

		@@ -9,6 +9,23 @@ Calling native code from .NET is powerful but unforgiving. Incorrect signatures,

		This skill covers both `DllImport` (available since .NET Framework 1.0) and `LibraryImport` (source-generated, .NET 7+). When targeting .NET Framework, always use `DllImport`. When targeting .NET 7+, prefer `LibraryImport` for new code. When native AOT is a requirement, `LibraryImport` is the only option.

		## When to Use This Skill

		@@ -137,6 +160,8 @@ internal static partial int SetName(string name);

		When memory crosses the boundary, exactly one side must own it — and both sides must agree.

		> ❌ NEVER free with a mismatched allocator — `Marshal.FreeHGlobal` on `malloc`'d memory is heap corruption.

		@@ -50,6 +75,10 @@ The most dangerous mappings — these cause the majority of bugs:

		For the complete type mapping table, struct layout, and blittable type rules, see [references/type-mapping.md](references/type-mapping.md).

		> ❌ NEVER use `int` or `long` for C `long` — it's 32-bit on Windows, 64-bit on Unix. Always use `CLong`.

	> ❌ NEVER use `bool` without `MarshalAs` — the default marshal size is wrong.
	> ⚠️ Do not assume the default `bool` marshalling matches native — confirm the native boolean type. For Win32 `BOOL`, the default is correct; for C99 `bool` / `_Bool`, use `[MarshalAs(UnmanagedType.U1)] bool`.

Conversation

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

Uh oh!

Builds on PR #54

Changes

What changed

Why

Evidence

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

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!

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

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!

danmoseley commented Feb 21, 2026

Uh oh!

lewing commented Feb 21, 2026

Uh oh!

AaronRobinsonMSFT commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffschwMSFT left a comment

Choose a reason for hiding this comment

Uh oh!

jeffschwMSFT commented Mar 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lewing commented Feb 20, 2026 •

edited

Loading

lewing Feb 21, 2026 •

edited

Loading

lewing Feb 23, 2026 •

edited

Loading