Skip to content

Commit 26840ad

Browse files
joshsmithxrmclaude
joshsmithxrm
and
claude
authored
docs: add Claude workflow automation and streamline CLAUDE.md (#83)
* docs: add Claude workflow automation and streamline CLAUDE.md - Add /pre-pr command for pre-commit validation (build, test, changelog check) - Add /review-bot-comments command for triaging bot review findings - Add pre-commit hook to validate builds/tests before commit - Update development workflow to reference new commands - Streamline CLAUDE.md (remove verbose namespace examples, simplify comments section) - Add testing requirements table showing coverage gaps (Auth, Migration need tests) - Add bot review handling guidance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: minor refinements to hook and CLAUDE.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: improve pre-commit hook robustness - Add warning message on JSON parse failure (#9) - Handle FileNotFoundError when dotnet not in PATH (#1, #12) - Add 5-minute timeout to subprocess calls (#11) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: improve pre-commit hook command detection and output - Use shlex.split for robust git commit detection (#2) - Print both stdout and stderr on build/test failure (#3, #4) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: simplify gh api permission in settings example 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f234c3e commit 26840ad

File tree

6 files changed

+287
-50
lines changed

6 files changed

+287
-50
lines changed

‎.claude/commands/pre-pr.md‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# Pre-PR Validation
2+
3+
Run before creating a PR to catch issues early.
4+
5+
## Usage
6+
7+
`/pre-pr`
8+
9+
## Checks Performed
10+
11+
### 1. Build & Test
12+
```bash
13+
dotnet build -c Release --warnaserror
14+
dotnet test --no-build -c Release
15+
```
16+
17+
### 2. New Public APIs
18+
- Check for public classes/methods without XML documentation
19+
- Check for new public APIs without corresponding tests
20+
21+
### 3. Common Issues
22+
- Generic catch clauses (`catch { }` or `catch (Exception)` without logging)
23+
- TODO/FIXME comments that should be issues
24+
- Console.WriteLine in library code (should use ILogger or AuthenticationOutput)
25+
- Dead code (unused private methods, unreachable code)
26+
27+
### 4. Changelog
28+
- Verify CHANGELOG.md in affected package(s) is updated
29+
- If not, prompt: "Update changelog for [package]?"
30+
31+
### 5. Test Coverage (New Code)
32+
- For each new class: does corresponding test file exist?
33+
- Prompt: "No tests for [ClassName]. Add tests?" → Generate test stubs
34+
35+
## Output
36+
37+
```
38+
Pre-PR Validation
39+
=================
40+
[x] Build: PASS
41+
[x] Tests: PASS (42 passed)
42+
[!] Missing XML docs: MsalClientBuilder.CreateClient()
43+
[x] No TODOs found
44+
[!] Changelog not updated for PPDS.Auth
45+
[!] No tests for: ThrottleDetector, MsalClientBuilder
46+
47+
Fix issues? [Y/n]
48+
```
49+
50+
## Behavior
51+
52+
- On first failure: stop and report
53+
- On warnings: list all, ask whether to fix
54+
- Auto-fix what's possible (changelog stubs, test file creation)
55+
- Manual fix guidance for others
56+
57+
## When to Use
58+
59+
- Before `git commit` for significant changes
60+
- Before `gh pr create`
61+
- After addressing bot review comments

‎.claude/commands/review-bot-comments.md‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
# Review Bot Comments
2+
3+
Systematically address PR review comments from Copilot, Gemini, and CodeQL.
4+
5+
## Usage
6+
7+
`/review-bot-comments [pr-number]`
8+
9+
## Process
10+
11+
### 1. Fetch Comments
12+
```bash
13+
gh api repos/joshsmithxrm/ppds-sdk/pulls/[PR]/comments
14+
```
15+
16+
### 2. Triage Each Comment
17+
18+
For each bot comment, determine:
19+
20+
| Verdict | Action |
21+
|---------|--------|
22+
| **Valid** | Fix the issue, reply "Fixed in [commit]" |
23+
| **False Positive** | Reply with reason, dismiss |
24+
| **Unclear** | Investigate before deciding |
25+
26+
### 3. Common False Positives
27+
28+
| Bot Claim | Why It's Often Wrong |
29+
|-----------|---------------------|
30+
| "Use .Where() instead of foreach+if" | Preference, not correctness |
31+
| "Volatile needed with Interlocked" | Interlocked provides barriers |
32+
| "OR should be AND" | Logic may be intentionally inverted (DeMorgan) |
33+
| "Static field not thread-safe" | May be set once at startup |
34+
35+
### 4. Common Valid Findings
36+
37+
| Pattern | Usually Valid |
38+
|---------|---------------|
39+
| Unused variable/parameter | Yes - dead code |
40+
| Missing null check | Check context |
41+
| Resource not disposed | Yes - leak |
42+
| Generic catch clause | Context-dependent |
43+
44+
## Output
45+
46+
```markdown
47+
## Bot Review Triage - PR #82
48+
49+
| # | Bot | Finding | Verdict | Action |
50+
|---|-----|---------|---------|--------|
51+
| 1 | Gemini | Use constants in dict | Valid | Fixed in abc123 |
52+
| 2 | Copilot | Add validation tests | Valid | Fixed in def456 |
53+
| 3 | Copilot | Use .Where() | False Positive | Style preference |
54+
| 4 | CodeQL | Generic catch | Valid (low) | Acceptable for disposal |
55+
56+
All findings addressed: Yes
57+
```
58+
59+
## When to Use
60+
61+
- After opening a PR (before requesting review)
62+
- After new bot comments appear
63+
- Before merging

‎.claude/hooks/pre-commit-validate.py‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Pre-commit validation hook for PPDS SDK.
4+
Runs dotnet build and test before allowing git commit.
5+
"""
6+
import json
7+
import shlex
8+
import subprocess
9+
import sys
10+
import os
11+
12+
def main():
13+
try:
14+
input_data = json.load(sys.stdin)
15+
except json.JSONDecodeError:
16+
print("⚠️ pre-commit-validate: Failed to parse input. Skipping validation.", file=sys.stderr)
17+
sys.exit(0)
18+
19+
tool_name = input_data.get("tool_name", "")
20+
tool_input = input_data.get("tool_input", {})
21+
command = tool_input.get("command", "")
22+
23+
# Only validate git commit commands (robust check using shlex)
24+
if tool_name != "Bash":
25+
sys.exit(0)
26+
try:
27+
parts = shlex.split(command)
28+
is_git_commit = len(parts) >= 2 and os.path.basename(parts[0]) == "git" and parts[1] == "commit"
29+
except ValueError:
30+
is_git_commit = False
31+
if not is_git_commit:
32+
sys.exit(0) # Allow non-commit commands
33+
34+
# Get project directory
35+
project_dir = os.environ.get("CLAUDE_PROJECT_DIR", os.getcwd())
36+
37+
try:
38+
# Run dotnet build
39+
print("Running pre-commit validation...", file=sys.stderr)
40+
build_result = subprocess.run(
41+
["dotnet", "build", "-c", "Release", "--nologo", "-v", "q"],
42+
cwd=project_dir,
43+
capture_output=True,
44+
text=True,
45+
timeout=300 # 5 minute timeout
46+
)
47+
48+
if build_result.returncode != 0:
49+
print("❌ Build failed. Fix errors before committing:", file=sys.stderr)
50+
if build_result.stdout:
51+
print(build_result.stdout, file=sys.stderr)
52+
if build_result.stderr:
53+
print(build_result.stderr, file=sys.stderr)
54+
sys.exit(2) # Block commit
55+
56+
# Run dotnet test (unit tests only - integration tests run on PR)
57+
test_result = subprocess.run(
58+
["dotnet", "test", "--no-build", "-c", "Release", "--nologo", "-v", "q",
59+
"--filter", "Category!=Integration"],
60+
cwd=project_dir,
61+
capture_output=True,
62+
text=True,
63+
timeout=300 # 5 minute timeout
64+
)
65+
66+
if test_result.returncode != 0:
67+
print("❌ Unit tests failed. Fix before committing:", file=sys.stderr)
68+
if test_result.stdout:
69+
print(test_result.stdout, file=sys.stderr)
70+
if test_result.stderr:
71+
print(test_result.stderr, file=sys.stderr)
72+
sys.exit(2) # Block commit
73+
74+
print("✅ Build and unit tests passed", file=sys.stderr)
75+
sys.exit(0) # Allow commit
76+
77+
except FileNotFoundError:
78+
print("⚠️ pre-commit-validate: dotnet not found in PATH. Skipping validation.", file=sys.stderr)
79+
sys.exit(0)
80+
except subprocess.TimeoutExpired:
81+
print("⚠️ pre-commit-validate: Build/test timed out after 5 minutes. Skipping validation.", file=sys.stderr)
82+
sys.exit(0)
83+
84+
if __name__ == "__main__":
85+
main()

‎.claude/settings.json‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"hooks": {
3+
"PreToolUse": [
4+
{
5+
"matcher": "Bash",
6+
"hooks": [
7+
{
8+
"type": "command",
9+
"command": "python \"$CLAUDE_PROJECT_DIR/.claude/hooks/pre-commit-validate.py\"",
10+
"timeout": 120
11+
}
12+
]
13+
}
14+
]
15+
}
16+
}

‎.claude/settings.local.example.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@
7979
"Bash(gh pr status:*)",
8080
"Bash(gh pr checkout:*)",
8181
"Bash(gh pr diff:*)",
82-
"Bash(gh api repos/*/pulls/*/comments:*)",
83-
"Bash(gh api repos/*/pulls/*/comments/*:*)",
82+
"Bash(gh api:*",
8483
"Bash(gh issue list:*)",
8584
"Bash(gh issue view:*)",
8685
"Bash(gh issue status:*)",

‎CLAUDE.md‎

Lines changed: 61 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,11 @@ dotnet clean
115115

116116
1. Create feature branch from `main`
117117
2. Make changes
118-
3. Run `dotnet build` and `dotnet test`
119-
4. Update `CHANGELOG.md`
120-
5. Create PR to `main`
118+
3. **Add tests for new classes** (no new code without tests)
119+
4. Update `CHANGELOG.md` (same commit, not after)
120+
5. Run `/pre-pr` before committing
121+
6. Create PR to `main`
122+
7. Run `/review-bot-comments` after bots comment
121123

122124
### Code Conventions
123125

@@ -144,57 +146,21 @@ public class PluginStepAttribute : Attribute { }
144146

145147
### Code Comments
146148

147-
Comments explain WHY, not WHAT. The code documents what it does.
149+
Comments explain WHY, not WHAT.
148150

149151
```csharp
150-
// ❌ Bad - explains what (the code already shows this)
151-
// Loop through all options and check if required
152+
// ❌ Bad - explains what
153+
// Loop through all options
152154
foreach (var option in command.Options)
153155

154-
// ❌ Bad - references external tool as justification
155-
// Use [Required] prefix like Azure CLI does
156-
option.Description = $"[Required] {desc}";
157-
158-
// ✅ Good - explains why (non-obvious side effect)
156+
// ✅ Good - explains why
159157
// Required=false hides the default suffix; we show [Required] in description instead
160158
option.Required = false;
161-
162-
// ✅ Good - explains why (workaround for framework limitation)
163-
// Option validators only run when the option is present on command line,
164-
// so we need command-level validation to catch missing required options
165-
command.Validators.Add(result => { ... });
166159
```
167160

168161
### Namespaces
169162

170-
```csharp
171-
// PPDS.Plugins
172-
namespace PPDS.Plugins; // Root
173-
namespace PPDS.Plugins.Attributes; // Attributes
174-
namespace PPDS.Plugins.Enums; // Enums
175-
176-
// PPDS.Dataverse
177-
namespace PPDS.Dataverse.Pooling; // Connection pool, IConnectionSource
178-
namespace PPDS.Dataverse.BulkOperations; // Bulk API wrappers
179-
namespace PPDS.Dataverse.Configuration; // Options, connection config
180-
namespace PPDS.Dataverse.Resilience; // Throttle tracking, service protection
181-
182-
// PPDS.Migration
183-
namespace PPDS.Migration.Export; // IExporter
184-
namespace PPDS.Migration.Import; // IImporter
185-
186-
// PPDS.Auth
187-
namespace PPDS.Auth.Profiles; // AuthProfile, ProfileStore, ProfileCollection
188-
namespace PPDS.Auth.Credentials; // ICredentialProvider, credential implementations
189-
namespace PPDS.Auth.Discovery; // GlobalDiscoveryService, EnvironmentResolver
190-
namespace PPDS.Auth.Cloud; // CloudEnvironment, CloudEndpoints
191-
192-
// PPDS.Cli
193-
namespace PPDS.Cli.Commands.Auth; // Auth command group
194-
namespace PPDS.Cli.Commands.Env; // Environment command group
195-
namespace PPDS.Cli.Commands.Data; // Data command group (export, import, copy)
196-
namespace PPDS.Cli.Infrastructure; // ServiceFactory, ProfileServiceFactory
197-
```
163+
Follow existing patterns: `PPDS.{Package}.{Area}` (e.g., `PPDS.Auth.Credentials`). Infer from code.
198164

199165
---
200166

@@ -411,10 +377,57 @@ See [CLI README](src/PPDS.Cli/README.md) for full documentation.
411377
412378
## 🧪 Testing Requirements
413379
414-
- **Target 80% code coverage**
415-
- Unit tests for all public API (attributes, enums)
416-
- Run `dotnet test` before submitting PR
417-
- All tests must pass before merge
380+
| Package | Test Project | Status |
381+
|---------|--------------|--------|
382+
| PPDS.Plugins | PPDS.Plugins.Tests | ✅ |
383+
| PPDS.Dataverse | PPDS.Dataverse.Tests | ✅ |
384+
| PPDS.Cli | PPDS.Cli.Tests | ✅ |
385+
| PPDS.Auth | **Needs test project** | ❌ |
386+
| PPDS.Migration | **Needs test project** | ❌ |
387+
388+
**Rules:**
389+
- New public class → must have corresponding test class
390+
- New public method → must have test coverage
391+
- Mark integration tests with `[Trait("Category", "Integration")]`
392+
393+
**Test filtering:**
394+
- **Commits:** Unit tests only (`--filter Category!=Integration`)
395+
- **PRs:** All tests including integration
396+
397+
---
398+
399+
## 🤖 Bot Review Handling
400+
401+
PRs get reviewed by Copilot, Gemini, and CodeQL. **Not all findings are valid.**
402+
403+
| Finding Type | Action |
404+
|--------------|--------|
405+
| Unused code, resource leaks, missing tests | Usually valid - fix |
406+
| "Use .Where()", style suggestions | Often preference - dismiss with reason |
407+
| Logic errors (OR/AND) | Verify manually - bots misread DeMorgan |
408+
409+
**Workflow:** After PR created, run `/review-bot-comments [PR#]` to triage.
410+
411+
---
412+
413+
## 🛠️ Claude Commands & Hooks
414+
415+
### Commands
416+
417+
| Command | Purpose |
418+
|---------|---------|
419+
| `/pre-pr` | Validate before PR (build, test, changelog) |
420+
| `/review-bot-comments [PR#]` | Triage bot review findings |
421+
| `/handoff` | Session summary (workspace) |
422+
| `/create-issue [repo]` | Create issue (workspace) |
423+
424+
### Hooks (Automatic)
425+
426+
| Hook | Trigger | Action |
427+
|------|---------|--------|
428+
| `pre-commit-validate.py` | `git commit` | Build + unit tests (skips integration), blocks if failed |
429+
430+
Hooks in `.claude/settings.json`. Pre-commit runs ~10s, keeps broken code out of commits.
418431
419432
---
420433

0 commit comments

Comments
 (0)