pre-pr-self-review

星标3

分支1

更新时间2026年7月12日 02:47

Self-review checklist - plus an agentic review pass (a read-only reviewer persona over the diff) - before opening a PR. Use before invoking `create-pr`, when reviewing your own draft, or when a reviewer flags issues that should have been caught earlier. Codifies recurring mistakes from multi-targeted polyfill work - missing tests for new public surface, unchecked length sums, null-pointer foot-guns from `MemoryMarshal.GetReference` on empty spans, drift from `ArgumentNullException.ThrowIfNull` and `checked()` conventions, TFM phrasing errors, and stale PR descriptions.

安装

用 Codex 或 Claude 帮你安装复制这段 Prompt，粘贴到 Codex、Claude 或其他助手里，让它检查 Skill 页面并帮你完成安装。

来源

JeremyKuhne

JeremyKuhne/madowaku

打开 GitHub 仓库查看创作者相关仓库

下载

相关职业SOC

基于 SOC 职业分类

软件质量保证分析师与测试员计算机与数学类职业·SOC 15-1253

文件资源管理器

2 个文件

SKILL.md

readonly

同仓库更多 Skills

同仓库

cswin32-com

JeremyKuhne/madowaku

Guides struct-based COM interop using CsWin32 - AOT-compatible raw pointer calls without [ComImport] or built-in CLR marshalling; managed [ComImport] mirrors are limited to CCW bridges. Consult when working with ComScope, IID.Get, delegate* unmanaged vtables, CoCreateInstance or a class factory, IComIID across .NET 10 and .NET Framework, connection-point Advise/Unadvise ownership, caller-owned CCW references, manually defining COM interfaces not in Win32 metadata (WMI, Fusion, CLR hosting/metadata), COM pointer lifetime in fields, cross-assembly CCWs, or mocking struct-based COM in tests. Paired with the cswin32-interop skill for the general P/Invoke layer and blittable-signature rules.

2026-07-173

cswin32-interop

JeremyKuhne/madowaku

Guides CsWin32 P/Invoke interop in a multi-targeted .NET 10 and .NET Framework library. Consult when replacing [DllImport] with source-generated PInvoke.* calls, working with generated Windows.Win32 projections (HANDLE / HMODULE / HRESULT / BOOL and typed enum/constant types), configuring NativeMethods.txt / NativeMethods.json, composing a public PInvoke across foundation/owner/extender packages with extensionReceiver, deciding which support library owns a helper, auditing native allocation and byte/element lengths, selecting compile-time / runtime / analyzer guards for Windows-only code, or choosing between CsWin32 and [LibraryImport]. Paired with the cswin32-com skill for the struct-based COM layer.

2026-07-173

dotnet-polyfills

JeremyKuhne/madowaku

Set up and use the standard .NET downlevel polyfill stack so a multi-targeted library can call modern BCL APIs on .NET Framework - PolySharp for compiler and language attributes, the official Microsoft downlevel NuGet packages (System.Memory, Microsoft.Bcl.Memory, Microsoft.Bcl.HashCode, Microsoft.IO.Redist, and the other Microsoft.Bcl.* / System.* backports), and the KlutzyNinja.Touki package's runtime polyfills layered on top. Use when adding a downlevel net472 / net481 target, choosing which package supplies a missing type before hand-rolling, configuring PolySharp, or checking whether an API is already polyfilled. For authoring a new hand-rolled polyfill inside a repo's own Framework tree, see polyfill-dotnet-api.

2026-07-123

publish-release

JeremyKuhne/madowaku

Publish a new version of the `KlutzyNinja.Madowaku` NuGet package by cutting a release tag. Use when asked to "publish a new version", "release alpha.N", "ship a beta", "cut a release", "promote alpha to beta", or "tag and publish". Walks through choosing the right `Major.Minor.Patch` bump, deciding whether to stay in `alpha` / `beta` / `rc` / stable, composing the `v*` tag, pushing it, and creating the matching GitHub release. Flags when an `AssemblyVersion`-changing Major bump is required (binary breaking changes vs additive/bugfix work).

2026-07-123

manage-skills

JeremyKuhne/madowaku

Find, add, update, and share agent skills in this repo. Use when asked to "find a skill" for a task, "build a skill" or "create a skill" (this skill checks whether one already exists - in the repo, the shared commons, or a public catalog - before authoring a new one), "update a skill", or reconcile a local skill change against the upstream commons vs a repo-local overlay. Covers the find-first build path, the tiered search, and the pull/push update flow. Not for validating one agent file's syntax - that is `agent-files-review`.

2026-07-113

address-pr-feedback

JeremyKuhne/madowaku

Address feedback on an existing pull request - review comments, requested changes, CI failures, or any post-PR follow-up work. Use when the user says "address the review", "fix the comments", "address Copilot's feedback", "fix the CI failure", or any similar phrasing. Distinct from `create-pr`, which covers opening the *initial* PR.

2026-07-113

compatibility	Requires git plus the repository's build, test, and agent-file validation commands.
description	Self-review checklist - plus an agentic review pass (a read-only reviewer persona over the diff) - before opening a PR. Use before invoking `create-pr`, when reviewing your own draft, or when a reviewer flags issues that should have been caught earlier. Codifies recurring mistakes from multi-targeted polyfill work - missing tests for new public surface, unchecked length sums, null-pointer foot-guns from `MemoryMarshal.GetReference` on empty spans, drift from `ArgumentNullException.ThrowIfNull` and `checked()` conventions, TFM phrasing errors, and stale PR descriptions.
license	MIT
metadata	{"applicability":"universal","binding":"optional-overlay","github-path":"skills/pre-pr-self-review","github-pinned":"v0.10.0","github-ref":"refs/tags/v0.10.0","github-repo":"https://github.com/JeremyKuhne/agent-skills","github-tree-sha":"880e117910c69bb909f59a3dfea81cbbf33ac047","maturity":"canary","portability":"portable","related":"create-pr, address-pr-feedback, security-review, performance-testing","requires":"none","risk":"local-write"}
name	pre-pr-self-review

Pre-PR self-review

If overlay.md exists beside this file, read it before acting; it contains repository-specific bindings. This core remains usable without it.

Run this checklist before invoking the create-pr skill. Each item is a question your code or PR body must answer. Update the skill whenever a reviewer flags something not yet listed.

This skill pairs with several others a consuming repo wires concretely in its overlay: a polyfill-dotnet-api skill (the source-preference and design rules this checklist validates), create-pr (the workflow this precedes), address-pr-feedback (the follow-up that re-runs this checklist), performance-testing (benchmark authoring required when a perf claim drives a change), framework-jit-optimization (net481 RyuJIT tradeoffs cited in the polyfill-correctness items), agent-files-review (for changes under .agents/, AGENTS.md, or .github/copilot-instructions.md), and security-review (the security-specific subset - abusive-input handling, length / integer overflow, allocation and algorithmic DoS, argument validation, and every use of unsafe / Unsafe.* / MemoryMarshal.* / Marshal.* or any BCL API whose docs say "unsafe" or "caller must"). Invoke security-review alongside this checklist for any change that adds or modifies a member accepting caller-supplied data, or that touches one of those caller-validated constructs - the common case, not a niche.

Agentic review pass

Before walking the checklist, run an automated review pass over the diff so the reviewer-bot class of findings - correctness edge cases, hand-rolled parser/format pitfalls, CI and supply-chain hygiene, and doc-vs-behavior drift - surfaces locally instead of across PR rounds. Spawn a read-only pre-PR reviewer persona (a consuming repo wires the concrete agent in its overlay) over the working diff, triage its findings (valid / nit / judgment call / likely false positive), fix the valid ones, and re-run at most once when the fixes were non-trivial. Keep it bounded: a same-class model nitpicks indefinitely, so two passes is the cap and the deterministic gates - tests, lint, the format validators - stay the source of truth. The checklist below is the human complement: the recurring, domain-specific mistakes an agent pass tends to miss.

1. Tests cover every new branch

For each new public (or InternalsVisibleTo-internal) member:

Search the test projects for the symbol; no hits = missing test.
Polyfills in the Framework-only tree: tests run on both TFMs. Wrap polyfill-only paths (subclass fallbacks, null-receiver guards) in #if NETFRAMEWORK.
Runtime type-check fast paths (typeof(T) == obj.GetType()): test the fast path and a subclass override.
Generic primitive specializations: every specialized branch needs a test. Don't rely on byte/int covering bool/sbyte/short/ ushort/uint/long/ulong - each has independent ref reinterpretation.
Security-sensitive APIs (FixedTimeEquals, hex decode): cover equal, differing-content, length mismatch, both empty, one empty, and a long span where only the last byte differs.
Allocating APIs (Concat, ToHexString): include an OverflowException test on the length sum.

Test hygiene for the tests themselves - the recurring miss list that costs the most review rounds on coverage-only PRs:

Test method names start with the method under test. MethodName_StateUnderTest_ExpectedBehavior per the repo's test conventions. ReadOnlySpan_Empty_ReturnsEmpty is wrong; SliceAtNull_ReadOnlySpan_Empty_ReturnsEmpty is right.
Every IDisposable test local uses using or try/finally. A temp-folder helper, a matcher handle, a pooled-list rental - a bare local leaks the resource when an assertion fails. Use the try/finally pattern when the test itself exercises explicit Dispose().
Don't hard-code InvariantCulture for APIs that use CurrentCulture. Provider-less formatting helpers generally format with CurrentCulture. Asserting against InvariantCulture-formatted strings makes the test locale-dependent.

2. Polyfill / framework correctness

For any change in the Framework-only tree (a polyfill or a framework-only fast path), walk these items (a consuming repo may keep the per-item detail and code patterns in a polyfill-correctness overlay companion):

Empty / null spans handled before unsafe interop (empty source, empty destination, both empty; exception type cross-checked).
Multi-input length sums wrapped in checked().
Throw helpers use the standard BCL exceptions, not custom types.
Span overloads stay allocation-free by default (document any trade-off in <remarks>).
Behavior parity with the modern BCL (exception type and message family, edge cases, type-exact fast paths).
Performance claims name the JIT (net481 RyuJIT vs modern .NET RyuJIT) and are measured or explicitly marked unmeasured.

If the change is not in the Framework-only tree, skip to §3.

3. PR description matches reality

TFM phrasing: name the polyfill's target TFM (the framework target, e.g. net472) distinctly from the TFM the tests merely run on (e.g. net481). Do not call a net472-targeted polyfill "net481-only".
File list, test counts, and perf numbers all reflect the current diff. Re-run after every commit; do not paste numbers from an earlier iteration.
Walk each bullet of the description against the diff before pushing. If the body says "covers Foo with cases A/B/C", search the diff for tests named Foo_… and confirm A, B, and C are all there. Review rounds have been lost to descriptions claiming a case (a Span<char> "null at end", a double-dispose test) that was not actually in the diff.
"Deliberately deferred" entries match what's actually absent from the working tree.

4. Final audit before staging

git status --short - delete leftover probe / scratch files; confirm every listed file belongs in the change set.
git diff --check - whitespace.
Rebase onto the canonical main if the branch trails it. Use upstream/main when working from a fork (the canonical repo lives at upstream), origin/main when cloning the canonical repo directly. Recently-merged sister PRs may have introduced files this PR cross-references; running off a stale base point makes those links look broken to an offline link check that gates .agents/**, AGENTS.md, and *.instructions.md changes. A PR once lost a review round to exactly this.
For changes that touch .agents/, AGENTS.md, or .github/copilot-instructions.md, also run the repo's agent-file link checker (see the agent-files-review skill for options, including changed-only and base-ref modes).
Build both TFMs.
Run the test suite in both Debug and Release. Release-mode RyuJIT inlining surfaces bugs Debug doesn't - e.g. [AggressiveInlining] + Unsafe.As<T, byte>(ref param) propagates the caller's int-promoted argument into the comparison immediate (cmp ecx, 0xFFFFFFFF instead of cmp ecx, 0xFF) for negative signed-primitive inputs on net481, but only in Release. Mask explicitly with & 0xFF / & 0xFFFF. See the polyfill-dotnet-api and framework-jit-optimization skills.
Stage by path, never git add -A / git add . when the working tree spans more than one logical change. If topics are intermingled, ask before staging.

5. Failing CI is a stop, not a sprint

When a build / test / CI run fails on a PR:

Diagnose.
Prepare the fix in the working tree.
Describe what changed, why, and any risks.
Stop and wait for explicit approval before commit/push.

Stacked rapid-fire fix commits are how perf regressions and unrelated sweep-ups get into history.

6. Update this skill

If a reviewer flags something not in this checklist, add it. If the review touched .agents/ files, also update via the agent-files-review workflow so the validator and any CI mirror stay in sync.