Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 57 additions & 24 deletions .claude/commands/review-bot-comments.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,47 +15,80 @@ gh api repos/joshsmithxrm/ppds-sdk/pulls/[PR]/comments

### 2. Triage Each Comment

For each bot comment, determine:
For each bot comment, determine verdict and rationale:

| Verdict | Action |
|---------|--------|
| **Valid** | Fix the issue, reply "Fixed in [commit]" |
| **False Positive** | Reply with reason, dismiss |
| **Unclear** | Investigate before deciding |
| Verdict | Meaning |
|---------|---------|
| **Valid** | Bot is correct, code should be changed |
| **False Positive** | Bot is wrong, explain why |
| **Unclear** | Need to investigate before deciding |

### 3. Common False Positives
### 3. Present Summary and WAIT FOR APPROVAL

**CRITICAL: Do NOT implement fixes automatically.**

Present a summary table to the user:

```markdown
## Bot Review Triage - PR #XX

| # | Bot | Finding | Verdict | Recommendation | Rationale |
|---|-----|---------|---------|----------------|-----------|
| 1 | Gemini | Missing Dispose | Valid | Add dispose call | Prevents resource leak |
| 2 | Copilot | Use .Where() | False Positive | Decline | Style preference |
```

**STOP HERE. Wait for user to review and approve before making ANY changes.**

Bot suggestions can be wrong (e.g., suggesting methods that don't exist on an interface).
Always get user approval, then verify changes compile before committing.

### 4. Implement Approved Changes

After user approval:
1. Make the approved code changes
2. **Build and verify** - `dotnet build` must succeed
3. Run tests to confirm no regressions
4. Commit with descriptive message

### 5. Reply to Each Comment Individually

After changes are committed, reply to each bot comment. Do NOT batch responses into a single PR comment.

```bash
# Reply to a specific review comment
gh api repos/joshsmithxrm/ppds-sdk/pulls/{pr}/comments \
-f body="Fixed in abc123" \
-F in_reply_to={comment_id}
```

**Important:** The `in_reply_to` parameter must be the comment ID (numeric), not a URL. Get IDs from the fetch step.

| Verdict | Reply Template |
|---------|----------------|
| Valid (fixed) | `Fixed in {commit_sha} - {brief description}` |
| Declined | `Declining - {reason}` |
| False positive | `False positive - {explanation}` |

## Common False Positives

| Bot Claim | Why It's Often Wrong |
|-----------|---------------------|
| "Use .Where() instead of foreach+if" | Preference, not correctness |
| "Volatile needed with Interlocked" | Interlocked provides barriers |
| "OR should be AND" | Logic may be intentionally inverted (DeMorgan) |
| "Static field not thread-safe" | May be set once at startup |
| "Call Dispose on X" | Interface may not actually implement IDisposable |

### 4. Common Valid Findings
## Common Valid Findings

| Pattern | Usually Valid |
|---------|---------------|
| Unused variable/parameter | Yes - dead code |
| Missing null check | Check context |
| Resource not disposed | Yes - leak |
| Resource not disposed | Yes - but verify interface first |
| Generic catch clause | Context-dependent |

## Output

```markdown
## Bot Review Triage - PR #82

| # | Bot | Finding | Verdict | Action |
|---|-----|---------|---------|--------|
| 1 | Gemini | Use constants in dict | Valid | Fixed in abc123 |
| 2 | Copilot | Add validation tests | Valid | Fixed in def456 |
| 3 | Copilot | Use .Where() | False Positive | Style preference |
| 4 | CodeQL | Generic catch | Valid (low) | Acceptable for disposal |

All findings addressed: Yes
```

## When to Use

- After opening a PR (before requesting review)
Expand Down
52 changes: 21 additions & 31 deletions .claude/hooks/pre-commit-validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,36 @@
"""
Pre-commit validation hook for PPDS SDK.
Runs dotnet build and test before allowing git commit.

Note: This hook is only triggered for 'git commit' commands via the
matcher in .claude/settings.json - no need to filter here.
"""
import json
import shlex
import subprocess
import sys
import os
import json

def main():
try:
input_data = json.load(sys.stdin)
except json.JSONDecodeError:
print("⚠️ pre-commit-validate: Failed to parse input. Skipping validation.", file=sys.stderr)
sys.exit(0)

tool_name = input_data.get("tool_name", "")
tool_input = input_data.get("tool_input", {})
command = tool_input.get("command", "")

# Only validate git commit commands (robust check using shlex)
if tool_name != "Bash":
sys.exit(0)
def main():
# Read stdin (Claude Code sends JSON with tool info)
try:
parts = shlex.split(command)
is_git_commit = len(parts) >= 2 and os.path.basename(parts[0]) == "git" and parts[1] == "commit"
except ValueError:
is_git_commit = False
if not is_git_commit:
sys.exit(0) # Allow non-commit commands
json.load(sys.stdin)
except (json.JSONDecodeError, EOFError):
pass # Input parsing is optional - matcher already filtered

# Get project directory
# Get project directory from environment or use current directory
project_dir = os.environ.get("CLAUDE_PROJECT_DIR", os.getcwd())

try:
print("🔨 Running pre-commit validation...", file=sys.stderr)

# Run dotnet build
print("Running pre-commit validation...", file=sys.stderr)
build_result = subprocess.run(
["dotnet", "build", "-c", "Release", "--nologo", "-v", "q"],
cwd=project_dir,
capture_output=True,
text=True,
timeout=300 # 5 minute timeout
timeout=300
)

if build_result.returncode != 0:
Expand All @@ -51,16 +40,16 @@ def main():
print(build_result.stdout, file=sys.stderr)
if build_result.stderr:
print(build_result.stderr, file=sys.stderr)
sys.exit(2) # Block commit
sys.exit(2)

# Run dotnet test (unit tests only - integration tests run on PR)
# Run unit tests only (integration tests run on PR)
test_result = subprocess.run(
["dotnet", "test", "--no-build", "-c", "Release", "--nologo", "-v", "q",
"--filter", "Category!=Integration"],
cwd=project_dir,
capture_output=True,
text=True,
timeout=300 # 5 minute timeout
timeout=300
)

if test_result.returncode != 0:
Expand All @@ -69,17 +58,18 @@ def main():
print(test_result.stdout, file=sys.stderr)
if test_result.stderr:
print(test_result.stderr, file=sys.stderr)
sys.exit(2) # Block commit
sys.exit(2)

print("✅ Build and unit tests passed", file=sys.stderr)
sys.exit(0) # Allow commit
sys.exit(0)

except FileNotFoundError:
print("⚠️ pre-commit-validate: dotnet not found in PATH. Skipping validation.", file=sys.stderr)
print("⚠️ dotnet not found in PATH. Skipping validation.", file=sys.stderr)
sys.exit(0)
except subprocess.TimeoutExpired:
print("⚠️ pre-commit-validate: Build/test timed out after 5 minutes. Skipping validation.", file=sys.stderr)
print("⚠️ Build/test timed out. Skipping validation.", file=sys.stderr)
sys.exit(0)


if __name__ == "__main__":
main()
4 changes: 2 additions & 2 deletions .claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
"hooks": {
"PreToolUse": [
{
"matcher": "Bash",
"matcher": "Bash(git commit:*)",
"hooks": [
{
"type": "command",
"command": "python \"$CLAUDE_PROJECT_DIR/.claude/hooks/pre-commit-validate.py\"",
"command": "python .claude/hooks/pre-commit-validate.py",
"timeout": 120
}
]
Expand Down
2 changes: 1 addition & 1 deletion .claude/settings.local.example.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"Bash(gh pr status:*)",
"Bash(gh pr checkout:*)",
"Bash(gh pr diff:*)",
"Bash(gh api:*",
"Bash(gh api:*)",
"Bash(gh issue list:*)",
"Bash(gh issue view:*)",
"Bash(gh issue status:*)",
Expand Down
3 changes: 3 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ updates:
dataverse:
patterns:
- "Microsoft.PowerPlatform.Dataverse.*"
fakexrmeasy:
patterns:
- "FakeXrmEasy*"
# Widen version constraints when needed (e.g., 1.1.* -> 1.2.*)
versioning-strategy: increase

Expand Down
71 changes: 71 additions & 0 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Integration Tests

on:
push:
branches: [main]
pull_request:
branches: [main]

jobs:
unit-tests:
name: Unit Tests
runs-on: windows-latest

steps:
- uses: actions/checkout@v6
with:
fetch-depth: 0

- name: Setup .NET
uses: actions/setup-dotnet@v5
with:
dotnet-version: |
8.0.x
9.0.x
10.0.x

- name: Restore dependencies
run: dotnet restore

- name: Build
run: dotnet build --configuration Release --no-restore

- name: Run Unit Tests (exclude integration)
run: dotnet test --configuration Release --no-build --verbosity normal --filter "Category!=Integration"

integration-tests:
name: Integration Tests
runs-on: windows-latest
# Only run on PRs from the same repo (not forks) or on main branch
if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository
environment: test-dataverse
needs: unit-tests

steps:
- uses: actions/checkout@v6
with:
fetch-depth: 0

- name: Setup .NET
uses: actions/setup-dotnet@v5
with:
dotnet-version: |
8.0.x
9.0.x
10.0.x

- name: Restore dependencies
run: dotnet restore

- name: Build
run: dotnet build --configuration Release --no-restore

- name: Run Integration Tests
env:
DATAVERSE_URL: ${{ secrets.DATAVERSE_URL }}
PPDS_TEST_APP_ID: ${{ secrets.PPDS_TEST_APP_ID }}
PPDS_TEST_CLIENT_SECRET: ${{ secrets.PPDS_TEST_CLIENT_SECRET }}
PPDS_TEST_TENANT_ID: ${{ secrets.PPDS_TEST_TENANT_ID }}
PPDS_TEST_CERT_BASE64: ${{ secrets.PPDS_TEST_CERT_BASE64 }}
PPDS_TEST_CERT_PASSWORD: ${{ secrets.PPDS_TEST_CERT_PASSWORD }}
run: dotnet test --configuration Release --no-build --verbosity normal --filter "Category=Integration"
13 changes: 11 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,14 @@ jobs:
- name: Build
run: dotnet build --configuration Release --no-restore

- name: Test
run: dotnet test --configuration Release --no-build --verbosity normal
- name: Test with Coverage
run: |
dotnet test --configuration Release --no-build --verbosity normal --filter "Category!=Integration" --collect:"XPlat Code Coverage" --results-directory ./coverage

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v5
with:
directory: ./coverage
flags: unittests
fail_ci_if_error: false # Don't fail build if upload fails
verbose: true
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,6 @@ nul
data.zip
data/
schema.xml

# Code coverage
coverage/
Loading
Loading