Skip to content

Commit c6a61bc

Browse files
Zangesclaude
andcommitted
Add code quality automation: analyzers, Dependabot auto-merge, PR labeler, multi-agent Claude review
Three layers of automation, each with its own ai-docs/implementations/ writedown. Code quality enforcement - Directory.Build.props: AnalysisLevel=latest-recommended + EnforceCodeStyleInBuild=true + GenerateDocumentationFile=true. Combined with the existing TreatWarningsAsErrors=true, the curated Microsoft analyzer set now fails the build (and CI). - New root .editorconfig tunes per-rule severities so the level-up is workable on a WPF + native-interop codebase (silences CA1051 on the Signal hot-path struct, CA1859 in WPF builder methods, IDE0051 around Win32 documentary constants, etc., each with a one-line rationale). - Surfaced 4 real correctness fixes in interop code where return values were being silently discarded: WaitForSingleObject (WaitableTickTimer.cs), InterceptionNative.Send (InterceptionInputBackend.cs), GetDpiForMonitor and SetWindowLong (MonitorInfo.cs / WindowStyles.cs). - ~120 files auto-fixed via dotnet format style (IDE0011 add-braces, IDE0005 unused-usings, IDE0044 add-readonly) and dotnet format whitespace (CRLF normalization). No semantic changes in the bulk pass. Process automation - New repo labels: dependencies, ci, feature, fix, internal, refactor, chore, tests, docs, skip-changelog, build, plus per-project area labels (ui, engine, input, persistence, app, virtualpad). Created via gh label create; unblocks the 6 open Dependabot PRs that were failing to apply the `dependencies` label. - .github/workflows/dependabot-automerge.yml -- auto-approves and enables GitHub's native auto-merge on patch + minor + security Dependabot PRs once CI passes. Major versions still require manual review. - .github/workflows/labeler.yml + .github/labeler.yml -- path-based labeling (touches src/Mouse2Joy.UI -> ui label, etc.) plus title-based Conventional Commit prefix labeling. Drives the existing release-notes categorization without manual labeling. Multi-agent Claude PR review - .github/workflows/claude-review.yml -- listens for the existing CI workflow's completion via workflow_run. Gates on: CI succeeded, not Dependabot, not draft, no skip-claude-review label, diff >= 10 lines, not docs/dependency-only. If the gate passes, runs the review pipeline. - .github/claude-review/ -- TypeScript review script using @anthropic-ai/sdk directly. Runs 8 reviewer agents in parallel (security, correctness, regression, ux, stability, convention-compliance, performance, code-reuse), each with a tool surface (get_file_diff, read_file, grep_repo, list_dir) so they pull only what they need from the repo rather than getting the full diff dumped in. CLAUDE.md is cached once via cache_control: ephemeral and hits the cache across all 8 agents. Findings are validated via zod and aggregated into a single GitHub review (event: COMMENT, advisory). - Model: Claude Opus 4.7 with adaptive thinking and effort: "high". Per-agent output schema enforced via output_config.format (json_schema), not just prompt instruction. - New skip-claude-review label as an opt-out per PR. - .claude/skills/review-pr/SKILL.md -- companion local skill that reuses the same 8 reviewer prompt files from .github/claude-review/reviewers/. Invoked from a Claude Code session (`/review-pr` or via natural language) and reports findings in the conversation instead of posting to GitHub. Single source of truth for the reviewer prompts across CI and interactive use. Manual setup required after merge (cannot be scripted): - Enable repo setting "Allow auto-merge" (Settings -> General -> Pull Requests). The dependabot-automerge workflow runs but is a no-op until this is on. - Add ANTHROPIC_API_KEY repo secret (Settings -> Secrets and variables -> Actions) for the multi-agent review pipeline. Without it, the workflow runs but posts a one-line "skipped" comment instead of reviewing. Verification - dotnet build Mouse2Joy.sln -c Release -- 0 errors, 0 warnings - dotnet format --verify-no-changes -- passes - dotnet test -- 259 passed, 0 failed - npm run typecheck in .github/claude-review/ -- clean - No-key smoke test of the review script -- correctly skips with one-line exit message Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5eb5a0b commit c6a61bc

113 files changed

Lines changed: 5226 additions & 467 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/skills/review-pr/SKILL.md

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
---
2+
name: review-pr
3+
description: Run the Mouse2Joy multi-angle review locally on a PR or the current branch. Spawns 8 review subagents (security, correctness, regression, ux, stability, convention-compliance, performance, code-reuse) using the same reviewer prompts as the CI pipeline. Reports findings in the conversation instead of posting to GitHub. Use when the user wants to review a PR before opening it, double-check a branch, or get a second opinion on changes without waiting for CI.
4+
---
5+
6+
# review-pr
7+
8+
This is the **interactive / local** counterpart to the CI workflow at
9+
[`.github/workflows/claude-review.yml`](../../../.github/workflows/claude-review.yml).
10+
Same 8 reviewer angles, same prompts, but it runs from a Claude Code session
11+
and reports findings as text in the conversation instead of posting a GitHub
12+
review.
13+
14+
## When to use
15+
16+
Trigger this skill when the user asks for things like:
17+
18+
- "Review this PR" / "review PR #12" / "give me a review of branch X"
19+
- "Check my branch before I open the PR"
20+
- "What would the CI reviewer flag on this?"
21+
- "Run the multi-agent review locally"
22+
23+
Do **not** trigger if the user just wants a quick code-quality scan of one
24+
file or a specific question about one change — those don't need 8 angles.
25+
26+
## What it does
27+
28+
1. Resolves the target: PR number (via `gh pr view <n>`), explicit branch
29+
name, or "the current branch" by default.
30+
2. Loads the 8 reviewer system prompts from
31+
[`.github/claude-review/reviewers/`](../../../.github/claude-review/reviewers/).
32+
These files are the **single source of truth** — they are also used by
33+
the CI workflow.
34+
3. Lists the changed files in the diff.
35+
4. Runs each reviewer as a `Task` subagent in turn (sequentially, not in
36+
parallel — the local skill optimizes for transparency in the conversation
37+
over throughput).
38+
5. Aggregates findings into a single response in the conversation, with one
39+
section per angle.
40+
41+
## How to run it
42+
43+
Follow these steps in order. The user is in this Claude Code session; you
44+
are the orchestrator.
45+
46+
### Step 1: Resolve the target
47+
48+
Look at the user's request:
49+
50+
- If they named a PR number (e.g. "review PR #8"): use that.
51+
- If they named a branch: use `git diff <base>...<branch>` against
52+
`origin/main`.
53+
- Otherwise: default to the current branch vs `origin/main`. Run
54+
`git rev-parse --abbrev-ref HEAD` and check that it isn't `main`. If it
55+
is, ask the user which branch / PR they meant.
56+
57+
### Step 2: Identify changed files
58+
59+
Run one of:
60+
61+
- For a PR: `gh pr view <n> --json files --jq '.files[].path'`
62+
- For a branch: `git diff --name-only origin/main...<branch>`
63+
64+
Hold this list — you'll pass it to every reviewer.
65+
66+
If the list is empty, say so and stop.
67+
68+
If the list is *only* docs (`ai-docs/**`, `**/*.md`, `LICENSE`) or *only*
69+
dependency files (`Directory.Packages.props`, `global.json`, csproj package
70+
references), tell the user "this matches the CI gate's skip rules and
71+
wouldn't be reviewed automatically — running anyway since you asked
72+
explicitly" and continue.
73+
74+
### Step 3: Load reviewer prompts
75+
76+
Read all 8 files from `.github/claude-review/reviewers/`:
77+
78+
- `security.md`
79+
- `correctness.md`
80+
- `regression.md`
81+
- `ux.md`
82+
- `stability.md`
83+
- `convention-compliance.md`
84+
- `performance.md`
85+
- `code-reuse.md`
86+
87+
You'll inject each one as the `Task` subagent's instructions.
88+
89+
### Step 4: Run reviewers sequentially
90+
91+
For each of the 8 reviewer prompts, invoke the `Task` tool with:
92+
93+
- `subagent_type: "general-purpose"`
94+
- `description`: short (e.g. "Security review of PR #8")
95+
- `prompt`: the full text of the reviewer prompt file, followed by:
96+
97+
```
98+
---
99+
100+
## This review
101+
102+
Target: <PR #N | branch <name>>
103+
Base: origin/main
104+
Head: <head ref>
105+
106+
Changed files:
107+
<bulleted list of paths>
108+
109+
Use Read / Grep / Glob / Bash (for `git diff` and `gh pr diff`) to
110+
investigate. Don't dump everything — pull only what you actually need
111+
for your angle. Output the structured JSON your instructions require.
112+
```
113+
114+
After each subagent returns, summarize its findings into the conversation
115+
under a short heading like `## Security (N findings)` so the user sees
116+
progress, then move on. Don't try to render the JSON literally — turn it
117+
into readable markdown:
118+
119+
- Summary as a sentence.
120+
- Critical findings as bullets with `path:L<line>` references.
121+
- Suggestions / nits collapsed under a "more" or omitted if low value (the
122+
user is reading in real-time; respect their attention).
123+
124+
### Step 5: Final wrap-up
125+
126+
After all 8 finish, post one short consolidated summary:
127+
128+
- "**Critical**: N findings" with a one-line description each.
129+
- "**Suggestions worth considering**: N" — list 3–5 of the most useful.
130+
- Token usage if you tracked it (optional).
131+
132+
Offer to dive deeper into any specific finding the user wants to discuss.
133+
134+
## Important constraints
135+
136+
- **Do NOT post to GitHub.** This skill is local-only. The CI workflow
137+
handles GitHub reviews. If the user wants the same review posted as a
138+
GitHub review, point them at the CI pipeline (push the branch, open
139+
the PR, CI runs the review automatically).
140+
- **Sequential, not parallel.** The CI script fan-outs 8 agents in
141+
parallel; this skill runs them one at a time so the user sees progress
142+
and can interrupt. If they want speed, the CI workflow exists.
143+
- **Reviewer prompts are the contract.** Don't paraphrase or summarize
144+
them when injecting into a subagent — pass the file content verbatim.
145+
This is what keeps CI review and local review in sync.
146+
- **Conventions referenced by the prompts** (`CLAUDE.md`, the
147+
"Things that are easy to get wrong" section, `ai-docs/implementations/`)
148+
are read by the subagents themselves via tool calls. You don't need to
149+
pre-load them; the agent will reach for them when relevant.
150+
151+
## What this skill does NOT do
152+
153+
- Open PRs / push branches / merge anything.
154+
- Modify files (it's a review, not a fix).
155+
- Run the .NET build or tests (CI does that; this skill is post-CI in
156+
spirit, even if there's no formal gate).
157+
- Replace [`/review`](https://docs.claude.com/claude-code) — that's the
158+
generic Claude Code review command. This skill is the Mouse2Joy-specific
159+
8-angle methodology.
160+
161+
## Example invocations
162+
163+
User: "Review my current branch."
164+
165+
You:
166+
1. `git rev-parse --abbrev-ref HEAD``feature/widget-rotation`.
167+
2. `git diff --name-only origin/main...feature/widget-rotation` → 4 files.
168+
3. Read the 8 reviewer prompts.
169+
4. Run each via `Task` in turn, posting per-angle summaries as you go.
170+
5. Final consolidated summary.
171+
172+
---
173+
174+
User: "Run the multi-agent review on PR #12."
175+
176+
You:
177+
1. `gh pr view 12 --json files,headRefName --jq ...`
178+
2. Same as above, but with PR-specific context.

.editorconfig

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# EditorConfig: https://editorconfig.org
2+
#
3+
# Pairs with <AnalysisLevel>latest-recommended</AnalysisLevel> +
4+
# <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild> in Directory.Build.props.
5+
#
6+
# Philosophy: keep correctness/perf rules on, silence stylistic rules that
7+
# would generate noise on a working WPF + native-interop codebase without
8+
# improving quality. Severities here override analyzer defaults at build
9+
# time. With TreatWarningsAsErrors=true, anything left at "warning" becomes
10+
# a build failure -- choose between "warning" (must fix) and "suggestion"
11+
# (IDE-only hint) carefully.
12+
13+
root = true
14+
15+
[*]
16+
charset = utf-8
17+
end_of_line = crlf
18+
insert_final_newline = true
19+
trim_trailing_whitespace = true
20+
indent_style = space
21+
indent_size = 4
22+
23+
[*.{yml,yaml,json,md}]
24+
indent_size = 2
25+
26+
[*.{csproj,props,targets,xaml}]
27+
indent_size = 2
28+
29+
[*.cs]
30+
# ---- C# language conventions ----
31+
# Use the .NET defaults; keep these as suggestions so they appear in the IDE
32+
# but never break the build.
33+
dotnet_style_qualification_for_field = false:suggestion
34+
dotnet_style_qualification_for_property = false:suggestion
35+
dotnet_style_qualification_for_method = false:suggestion
36+
dotnet_style_qualification_for_event = false:suggestion
37+
csharp_style_var_when_type_is_apparent = true:suggestion
38+
csharp_style_var_elsewhere = false:suggestion
39+
40+
# ---- IDE rule severities (style) ----
41+
# Things we genuinely want enforced.
42+
dotnet_diagnostic.IDE0005.severity = warning # remove unnecessary usings
43+
dotnet_diagnostic.IDE0011.severity = warning # add braces (we already use them)
44+
dotnet_diagnostic.IDE0044.severity = warning # add readonly modifier
45+
dotnet_diagnostic.IDE0051.severity = warning # remove unused private members
46+
dotnet_diagnostic.IDE0052.severity = warning # remove unread private members
47+
48+
# Things we explicitly do NOT want as errors -- stylistic or churny.
49+
dotnet_diagnostic.IDE0008.severity = silent # use explicit type (taste)
50+
dotnet_diagnostic.IDE0058.severity = silent # expression value never used (fluent assertions)
51+
dotnet_diagnostic.IDE0066.severity = suggestion # use switch expression
52+
dotnet_diagnostic.IDE0072.severity = suggestion # add missing cases to switch expression
53+
dotnet_diagnostic.IDE0290.severity = silent # use primary constructor (churny for existing code)
54+
55+
# ---- Code-quality (CA) rule severities ----
56+
# Keep on as warnings (will be errors due to TreatWarningsAsErrors=true).
57+
# Explicitly silence the ones that don't fit a desktop / native-interop app.
58+
59+
dotnet_diagnostic.CA1062.severity = silent # validate args (we use nullable refs at boundaries)
60+
dotnet_diagnostic.CA1805.severity = silent # member initialized to default value (we use it to document persisted defaults on records)
61+
dotnet_diagnostic.CA1303.severity = silent # do not pass literals as localized parameters (no l10n)
62+
dotnet_diagnostic.CA1304.severity = silent # specify CultureInfo (desktop app, system culture is fine)
63+
dotnet_diagnostic.CA1305.severity = silent # specify IFormatProvider (ditto)
64+
dotnet_diagnostic.CA1307.severity = silent # specify StringComparison for clarity (ordinal everywhere by default)
65+
dotnet_diagnostic.CA1310.severity = warning # specify StringComparison for correctness
66+
dotnet_diagnostic.CA1311.severity = silent # specify culture for ToUpper/ToLower
67+
dotnet_diagnostic.CA1716.severity = silent # identifiers should not match keywords
68+
dotnet_diagnostic.CA1724.severity = silent # type names should not match namespaces
69+
dotnet_diagnostic.CA1812.severity = silent # internal class never instantiated (DI containers / WPF)
70+
dotnet_diagnostic.CA1822.severity = suggestion # member can be static (often false-positive for view-models)
71+
dotnet_diagnostic.CA1848.severity = silent # use LoggerMessage delegates (Serilog templates already cached)
72+
dotnet_diagnostic.CA1851.severity = warning # multiple enumerations of IEnumerable
73+
dotnet_diagnostic.CA1860.severity = warning # prefer Count == 0 over !Any()
74+
dotnet_diagnostic.CA2007.severity = silent # ConfigureAwait(false) (WPF app context)
75+
dotnet_diagnostic.CA1051.severity = silent # public readonly fields on hot-path structs (Signal, etc.) are deliberate
76+
77+
# WPF + Win32 P/Invoke realities.
78+
dotnet_diagnostic.CA1031.severity = suggestion # catch general exception types (host code legitimately does this)
79+
dotnet_diagnostic.CA1416.severity = silent # platform compatibility (whole app is win-only)
80+
dotnet_diagnostic.CA1707.severity = silent # underscore in identifiers (Win32 constants / test names)
81+
dotnet_diagnostic.CA1711.severity = silent # identifier suffix rules
82+
dotnet_diagnostic.CA2101.severity = silent # specify marshaling for P/Invoke (LPWStr default is fine)
83+
84+
# Tests: relax a few more things that don't add value in a test project.
85+
[**/*.Tests/**.cs]
86+
dotnet_diagnostic.CA1707.severity = silent # MethodName_With_Underscores is the test convention
87+
dotnet_diagnostic.CA1861.severity = silent # avoid constant arrays as args (test data)
88+
dotnet_diagnostic.CA2007.severity = silent
89+
dotnet_diagnostic.CA1859.severity = silent # return concrete type for perf (tests intentionally use abstractions)
90+
dotnet_diagnostic.IDE0051.severity = silent # unused private member (test helpers paired with peer fixtures)

0 commit comments

Comments
 (0)