feat(ci): add diffray configuration for AI-assisted code review#794
Conversation
Implements comprehensive diffray configuration with 22 project-specific rules: - PowerShell patterns (6 rules) - enforces ADR-005 - Workflow constraints (5 rules) - enforces ADR-006 - Agent templates (4 rules) - protects generated files - Session protocol (7 rules) - enforces governance Configuration focuses on architectural and security review where AI adds most value, while excluding areas covered by existing tools (PSScriptAnalyzer, markdownlint). Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
PR Validation ReportNote ✅ Status: PASS Description Validation
QA Validation
⚡ Warnings
Powered by PR Validation workflow |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive configuration for diffray AI-assisted code review, which is a great step towards automating architectural and security checks. The configuration is well-structured, with rules logically categorized and aligned with project ADRs. I've identified a few issues in the rule definitions that could impact their effectiveness: a critical missing content_regex that would cause numerous false positives, a security-related issue in a PowerShell example that contradicts the style guide, and a conflicting example between two workflow rules. Addressing these points will enhance the accuracy and consistency of the AI reviews.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive diffray configuration for AI-assisted code review, implementing 22 project-specific rules organized into 4 categories. The configuration focuses on enforcing architectural decisions (ADRs), security patterns, and quality gates where AI review adds value beyond existing linting tools.
Key Changes
- Diffray configuration with model settings, file filters, and tag-based rule selection tuned for this repository's existing tooling
- 22 custom rules covering PowerShell patterns, GitHub workflow constraints, agent template protection, and session protocol enforcement
- Documentation providing configuration guidance, rule descriptions, and integration notes
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
.diffray/config.yaml |
Main configuration with model settings (sonnet), confidence thresholds (70), tag filtering, and file exclusions |
.diffray/rules/powershell-patterns.yaml |
5 PowerShell-specific rules enforcing ADR-005, CmdletBinding usage, error handling, and parameter validation |
.diffray/rules/workflow-constraints.yaml |
5 GitHub Actions rules enforcing ADR-006 (thin workflows), error handling, and secret safety |
.diffray/rules/agent-templates.yaml |
4 agent template rules preventing direct edits to generated files and validating template structure |
.diffray/rules/session-protocol.yaml |
7 governance rules enforcing ADR-014 (read-only HANDOFF.md), skill usage, and commit practices |
.diffray/README.md |
Comprehensive documentation covering configuration structure, rule categories, customization, and testing guidance |
Comments suppressed due to low confidence (1)
.diffray/rules/agent-templates.yaml:158
- The rule
agent_frontmatter_validationhas tags "agent-system" and "configuration", neither of which are in the config.yaml "only" list. This means the rule will never run. Consider adding one of the allowed tags (such as "maintainability" or "architecture").
tags:
- agent-system
- configuration
Adds 6 new rule categories based on patterns from .agents/ and .serena/memories/: - Security patterns (4 rules): Path traversal, command injection, blind suppression, AI output - Git discipline (3 rules): No --no-verify, branch verification, commit threshold - User-facing content (3 rules): No internal PR/Issue/path references - Testing patterns (3 rules): Pester coverage, test-first, validation-before-retrospective - Naming conventions (3 rules): ADR, session log, skill format validation - Extended workflow (2 rules): Author association check, rate limit precheck Total rules: 40 (22 original + 18 new) Total configuration: 1,713 lines across 10 rule files Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a new .diffray configuration, README, and multiple rulesets covering agent templates, PowerShell patterns, workflows, session/PR/ADR protocols, git discipline, naming, security, testing, and user-facing content. Only documentation and YAML rules added; no executable code changed. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro 📒 Files selected for processing (12)
Comment |
Review Triage RequiredNote Priority: NORMAL - Human approval required before bot responds Review Summary
Next Steps
Powered by PR Maintenance workflow - Add triage:approved label |
- Add content_regex to session_path_normalization rule to prevent false positives - Use argument splatting in ps_error_handling_required example per security style guide - Remove inline shell logic from wf_error_handling example to avoid contradiction Addresses feedback from gemini-code-assist[bot] review. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
- Correct PowerShell rule count from 6 to 5 throughout README - Fix relative paths in README to use ../ prefix for cross-directory links - Add missing tags to config.yaml tags.only list (quality, consistency, git, testing, powershell, ci-cd) - Remove file filter exclusions for .agents/sessions and .agents/retrospective to allow rule matching - Update agent-templates rule tags from agent-system/documentation to consistency/maintainability/quality Fixes critical tag filtering issue where highest-importance rules (git_no_verify_prohibited: 10) and entire rule categories were excluded. All 40 rules now properly match tag filters. Addresses feedback from Copilot and cursor[bot] reviews. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Regex pattern improvements: - Add word boundaries to prevent false matches in command patterns - Fix npx|node|python|ruby|perl|bash|sh to use \b boundaries - Fix AI|LLM|GPT|Claude|Copilot to use \b boundaries - Fix git commands (commit/push/reset) to use \b boundaries - Fix gh commands (pr/issue) to use \b boundaries - Improve --no-verify pattern to check word boundaries - Fix workflow patterns to use non-capturing groups - Replace greedy .* with \S+ in for-loop pattern Code example improvements: - Fix CWE attribution: Replace Python-specific lgtm[py/path-injection] with PowerShell-appropriate nosec - Add inline error checking example to wf_error_handling (shows exit code check alongside script delegation) - Make Session pattern more specific: 'See Session \d+ for' to reduce false positives Addresses all regex and pattern feedback from Copilot review. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (7)
.diffray/README.md (1)
92-94: README documents exclusions not present in config.yaml.Lines 92-93 state
.agents/sessions/**and.agents/retrospective/**are excluded, butconfig.yamlonly excludes.serena/memories/**. Either update the README to match actual config, or add these paths toconfig.yaml.Past review flagged this same issue. The mismatch remains.
.diffray/rules/testing-patterns.yaml (1)
10-13: Redundant exclusion pattern.Line 13
!**/*.Tests.ps1is unnecessary since.psm1files would never match*.Tests.ps1. Harmless but adds noise..diffray/rules/workflow-constraints.yaml (1)
14-19: Unresolved:if\\s*\\[pattern is overly broad.This was flagged in a prior review. The pattern matches legitimate PowerShell conditionals like
if ($x) [...], not just bash-styleif [ ... ]. Consider narrowing to bash-specific syntax:content_regex: - - "if\\s*\\[" + - "if\\s+\\[\\s+.*?\\s+\\]\\s*;\\s*then".diffray/rules/powershell-patterns.yaml (2)
60-61: Unresolved: Regex only matches PascalCase function names.The pattern
function\\s+[A-Z]misses functions starting with lowercase. While PowerShell convention is PascalCase, non-conforming functions still need[CmdletBinding()]. Prior review suggestedfunction\\s+\\w.content_regex: - - "function\\s+[A-Z]" + - "function\\s+\\w"
210-212: Unresolved: Regex misses[Parameter(Mandatory=$true)]variants.The pattern
\\[Parameter\\(Mandatory\\)\\]is too specific. It won't match[Parameter(Mandatory=$true)]or[Parameter(Mandatory = $true)]. Prior review suggested a flexible pattern.content_regex: - - "\\[Parameter\\(Mandatory\\)\\]" + - "\\[Parameter\\([^)]*Mandatory".diffray/rules/session-protocol.yaml (2)
98-101: Unresolved: Tags will cause rule to be filtered out.This rule uses
pr-level,documentation,traceabilitytags. Ifconfig.yaml'stags.onlyallowlist doesn't include these, the rule is silently ignored despitealways_run: true. Prior review flagged this. Addqualitytag to match the agent type, or update the config allowlist.tags: - pr-level - documentation - traceability + - quality
192-195: Unresolved: Tags will cause rule to be filtered out.Same issue as
session_pr_template_required. Tagspr-level,traceability,documentationmay not be in config allowlist. Addqualitytag.tags: - pr-level - traceability - documentation + - quality
🧹 Nitpick comments (2)
.diffray/config.yaml (1)
10-12: Comment could be clearer about multiple artifact paths.Comment says "Build artifacts" but you're excluding both
artifacts/**(top-level) andbuild/artifacts/**. Consider clarifying or consolidating.Minor readability nit.
.diffray/rules/workflow-constraints.yaml (1)
70-74: Patternrun:\\s*\\|triggers on all multiline run blocks.This is intentionally broad, but nearly every workflow uses multiline
run:blocks for legitimate single-line orchestration. Expect high noise. Consider adding a line-count heuristic in the checklist or narrowing the regex to multi-statement patterns.
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.diffray/README.md.diffray/config.yaml.diffray/rules/agent-templates.yaml.diffray/rules/git-discipline.yaml.diffray/rules/naming-conventions.yaml.diffray/rules/powershell-patterns.yaml.diffray/rules/security-patterns.yaml.diffray/rules/session-protocol.yaml.diffray/rules/testing-patterns.yaml.diffray/rules/user-facing-content.yaml.diffray/rules/workflow-constraints.yaml.diffray/rules/workflow-extended.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- .diffray/rules/user-facing-content.yaml
- .diffray/rules/workflow-extended.yaml
🔇 Additional comments (18)
.diffray/config.yaml (1)
31-54: Rules configuration looks good after fixes.Tag filtering now includes the necessary tags for all 40 rules to execute. The
always_run: truepattern ingit_commit_count_thresholdproperly bypasses filtering when needed..diffray/rules/naming-conventions.yaml (1)
1-119: Rules properly structured and will execute.All three naming rules include
consistencytag which matches the config.yaml allowlist. File globs and patterns are well-defined..diffray/rules/agent-templates.yaml (1)
1-157: Rules properly aligned with tag filters after fixes.All four agent template rules now use tags from the config.yaml allowlist (architecture, consistency, maintainability, quality). Rule definitions are complete with proper checklists and examples.
.diffray/rules/security-patterns.yaml (1)
15-16: StartsWith pattern may miss common PowerShell usage.The pattern
\\.StartsWith\\(requires a literal dot beforeStartsWith(. In PowerShell, typical usage is$path.StartsWith($base)where the dot comes after a variable. The pattern will match this correctly since the dot is literal in the code.Past comment suggested
\\$\\w+\\.StartsWith\\s*\\(for stricter matching, but current pattern is simpler and catches the method call. Either approach works..diffray/rules/testing-patterns.yaml (1)
90-133: Retrospective rule properly targets non-excluded paths.After config.yaml fixes,
.agents/retrospective/**is no longer excluded from review. This rule will execute against its intended targets..diffray/rules/git-discipline.yaml (2)
100-143: Rule usesalways_run: trueto bypass tag filtering.The
git_commit_count_thresholdrule has tags that don't match the config allowlist, butalways_run: trueat line 108 ensures it executes regardless. This is intentional design for PR-level checks.Past comment about tag mismatch is addressed by this mechanism.
1-49: Highest importance rule (10) properly configured.
git_no_verify_prohibitedhas correct tags ("git", "quality") matching the allowlist. Regex pattern with word boundaries prevents false matches..diffray/rules/workflow-constraints.yaml (3)
110-143:wf_max_line_limitlacks enforcement mechanism.The rule has no
content_regex, so it triggers on every workflow file. The checklist says "count total lines" but Diffray rules can't programmatically count lines. This becomes a manual reminder rather than a detection rule.If Diffray supports custom scripts or external checks, consider integrating actual line counting. Otherwise, document in the README that this rule requires manual verification.
145-187: Error handling rule looks good after fixes.The examples now show both script delegation (preferred) and inline error checking with
if (-not $?). Word boundaries on the regex prevent false matches.
189-231: Secrets exposure rule is well-structured.Pattern
secrets\\.appropriately triggers on any secrets usage for review. The checklist covers the key vectors (echo, CLI args, verbose logging). Good balance of sensitivity vs. actionability..diffray/rules/powershell-patterns.yaml (3)
2-45:ps_no_bash_python_scriptsrule is solid.Exclusions for
.githooks/**and.github/actions/**/*.shhandle legitimate exceptions. The ADR-005 reference provides clear rationale.
94-152: Error handling rule with splatting example looks good.The good example now demonstrates proper argument splatting (
$ghArgs = @(...)withgh @ghArgs),if (-not $?)for exit code checking, and[Parameter(Mandatory)]. Aligns with security style guide.
154-196: Write-Host detection in modules is valid.Targeting
.psm1andscripts/**/*.ps1is appropriate. The checklist correctly suggestsWrite-VerboseandWrite-Informationas alternatives..diffray/rules/session-protocol.yaml (5)
2-37:session_handoff_readonlyrule is well-defined.Simple file glob targeting
.agents/HANDOFF.mdwith clear ADR-014 reference. No regex needed—any modification to this file triggers review.
39-76: Skill usage enforcement looks good after regex fixes.Word boundaries (
\\bgh\\s+pr\\b,\\bgh\\s+issue\\b) prevent false matches. Exclusion of.claude/skills/**avoids flagging the skills themselves.
104-138: ADR review trigger rule is appropriate.Targeting
ADR-*.mdin both.agents/architecture/anddocs/architecture/covers common locations. Thearchitecturetag should match config allowlist.
140-170: Commit atomicity rule relies on manual judgment.With
always_run: trueandfile_glob: '**/*', this triggers on every PR. The checklist describes manual git log inspection. This is guidance rather than detection, which is fine for advisory rules.
198-238: Path normalization rule fixed with content_regex.Now properly triggers only when absolute path patterns (
C:\\,D:\\,/Users/,/home/) are detected. Single-quoted YAML strings handle backslash escaping correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.diffray/README.md (1)
90-95: README documents exclusions that don't exist in config.Lines 92-94 state
.agents/sessions/**and.agents/retrospective/**are excluded, butconfig.yamlonly excludes.serena/memories/**. These paths were intentionally removed from exclusions so rules likename_session_log_formatandtest_no_retrospective_without_validationcan target them.Update the README to reflect actual configuration.
Proposed fix
### Filters Excluded paths: -- `.agents/sessions/**` - Historical session logs -- `.agents/retrospective/**` - Historical retrospectives - `.serena/memories/**` - AI memory (managed separately) +- `artifacts/**` - Build artifacts +- `**/test-data/**` - Test fixtures
🧹 Nitpick comments (2)
.diffray/rules/workflow-constraints.yaml (1)
70-103: wf_should_call_modules pattern is broad.The
run:\s*\|regex (line 74) matches any multiline run block. The checklist clarifies "more than 5 lines" but the pattern itself will flag short blocks too. Consider documenting this as intentional for reviewer triage, or making the match more targeted if false positives become noisy..diffray/rules/powershell-patterns.yaml (1)
206-212: Mandatory pattern may miss common variations.Line 212's regex
\[Parameter\(Mandatory\)\]only matches the exact attribute syntax. It won't catch:
[Parameter(Mandatory=$true)][Parameter(Mandatory = $true)]Consider broadening to
\[Parameter\([^)]*Mandatoryto catch all mandatory parameter declarations.Proposed fix
content_regex: - - "\\[Parameter\\(Mandatory\\)\\]" + - "\\[Parameter\\([^)]*Mandatory"
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.diffray/README.md.diffray/config.yaml.diffray/rules/agent-templates.yaml.diffray/rules/git-discipline.yaml.diffray/rules/naming-conventions.yaml.diffray/rules/powershell-patterns.yaml.diffray/rules/security-patterns.yaml.diffray/rules/session-protocol.yaml.diffray/rules/testing-patterns.yaml.diffray/rules/user-facing-content.yaml.diffray/rules/workflow-constraints.yaml.diffray/rules/workflow-extended.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- .diffray/rules/session-protocol.yaml
- .diffray/rules/naming-conventions.yaml
- .diffray/rules/git-discipline.yaml
- .diffray/rules/security-patterns.yaml
- .diffray/rules/agent-templates.yaml
- .diffray/rules/workflow-extended.yaml
🔇 Additional comments (6)
.diffray/config.yaml (1)
1-54: Configuration looks solid after fixes.The tag filtering now includes all necessary tags (quality, consistency, git, testing, powershell, ci-cd) so all 40 rules will match. The exclusion patterns are reasonable for this project's structure.
One minor nit: Line 10 comment says "Build artifacts" but includes both root-level
artifacts/**and nestedbuild/artifacts/**. Not wrong, just slightly ambiguous..diffray/rules/user-facing-content.yaml (1)
1-129: Rules are well-defined.The three user-facing content rules properly target markdown files in src/ and templates/. Regex patterns are correctly escaped for YAML. Importance levels (8-9) appropriately flag these as high-priority content issues.
.diffray/rules/testing-patterns.yaml (1)
1-133: Testing pattern rules look good.Rules enforce Pester coverage for modules and test-first development. The
test_no_retrospective_without_validationrule is a clever process check to prevent "victory lap before finish line" anti-pattern.The
!**/*.Tests.ps1exclusion on line 13 is technically redundant (a.Tests.ps1file won't match*.psm1), but it's harmless and documents intent..diffray/rules/workflow-constraints.yaml (1)
1-231: Workflow rules are comprehensive.The 5 rules cover inline logic detection, module delegation, line limits, error handling, and secret exposure. Examples are clear and the ADR-006 references provide context. The error handling rule now correctly shows both script delegation and inline checking patterns.
.diffray/rules/powershell-patterns.yaml (1)
1-247: PowerShell rules enforce ADR-005 well.The 5 rules cover language compliance, CmdletBinding, error handling, testable output, and parameter validation. The splatting example in
ps_error_handling_requiredproperly models the security style guide. Exception patterns for git hooks and external actions (lines 15-16) are sensible..diffray/README.md (1)
1-334: Documentation is thorough.The README covers configuration structure, rule categories with importance levels, testing guidance, and integration notes. The rule template and customization sections will help future contributors. Relative paths to ADRs and constraints are now correct.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
.diffray/README.md (1)
92-93: Documentation claims exclusions not present in config.yaml.These paths aren't in the actual configuration file. Either remove these lines or add the exclusions to
config.yaml. Users will expect these directories to be excluded based on this documentation..diffray/rules/git-discipline.yaml (1)
134-137: Rule won't run due to tag mismatch.Tags
git-history,process, andautonomous-executionaren't inconfig.yaml's allowlist. The allowlist hasgitbut notgit-history. Addgitto this rule's tags or addgit-historyto the config allowlist. Without this, the rule is silently skipped despite being documented as active in README.🔎 Fix options
Option 1: Add
gittag to ruletags: + - git - git-history - process - autonomous-executionOption 2: Add
git-historyto config.yaml allowlist
Checkconfig.yamland addgit-historyto thetags.onlylist..diffray/rules/powershell-patterns.yaml (2)
60-61: Regex only matches PascalCase function names.The pattern
function\s+[A-Z]misses functions starting with lowercase letters. While PowerShell convention is PascalCase, this could miss legitimate functions that still need[CmdletBinding()].Consider:
function\s+\wto catch all function declarations.🔎 Proposed fix
content_regex: - - "function\\s+[A-Z]" + - "function\\s+\\w"
211-212: Regex won't match[Parameter(Mandatory=$true)]variants.The pattern
\[Parameter\(Mandatory\)\]is too strict. It misses common variations like[Parameter(Mandatory=$true)]or[Parameter(Mandatory = $true)].Use
\[Parameter\([^)]*Mandatoryto catch all mandatory parameter forms.🔎 Proposed fix
content_regex: - - "\\[Parameter\\(Mandatory\\)\\]" + - "\\[Parameter\\([^)]*Mandatory".diffray/rules/session-protocol.yaml (2)
98-101: Rule filtered out by tag mismatch.Tags
pr-level,documentation,traceabilityaren't in config.yaml'stags.onlylist. Despitealways_run: true, this rule won't execute.Add
qualityto align with the agent type and ensure the rule runs.🔎 Proposed fix
tags: - pr-level - documentation - traceability + - quality
192-195: Rule filtered out by tag mismatch.Same issue as
session_pr_template_required. Tagspr-level,traceability,documentationaren't in the allowlist.Add
qualityto match the agent and enable execution.🔎 Proposed fix
tags: - pr-level - traceability - documentation + - quality
🧹 Nitpick comments (2)
.diffray/rules/session-protocol.yaml (1)
131-133: Taggovernancenot in config.yaml allowlist.
session_adr_changes_trigger_reviewuses taggovernancewhich isn't intags.only. The rule still runs becausearchitectureis present, but consider addinggovernanceto the config or removing it here for consistency..diffray/config.yaml (1)
10-12: Minor: Comment could be clearer.Comment says "Build artifacts" but patterns cover both root-level
artifacts/**andbuild/artifacts/**. Not a functional issue.🔎 Clarified comment
- # Build artifacts + # Build artifacts (root and nested) - 'artifacts/**' - 'build/artifacts/**'
📜 Review details
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.diffray/README.md.diffray/config.yaml.diffray/rules/agent-templates.yaml.diffray/rules/git-discipline.yaml.diffray/rules/naming-conventions.yaml.diffray/rules/powershell-patterns.yaml.diffray/rules/security-patterns.yaml.diffray/rules/session-protocol.yaml.diffray/rules/testing-patterns.yaml.diffray/rules/user-facing-content.yaml.diffray/rules/workflow-constraints.yaml.diffray/rules/workflow-extended.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- .diffray/rules/testing-patterns.yaml
- .diffray/rules/agent-templates.yaml
- .diffray/rules/security-patterns.yaml
🔇 Additional comments (4)
.diffray/rules/powershell-patterns.yaml (1)
1-247: PowerShell patterns configuration looks solid.The five rules cover key areas: language enforcement (ADR-005), CmdletBinding, error handling, Write-Host avoidance, and parameter validation. Examples are clear. Tags align with config.yaml's
tags.onlylist..diffray/rules/workflow-extended.yaml (1)
1-122: Workflow extended rules look good.Both rules properly target
.github/workflows/*.yml. The author association check correctly uses$GITHUB_OUTPUTwith step conditionals (fixed from earlierexit 0issue). Rate limit precheck provides a practical threshold check. Tags match config.yaml's allowlist..diffray/rules/session-protocol.yaml (1)
1-238: Session protocol rules are well-structured overall.The seven rules cover handoff protection, skill usage enforcement, PR templates, ADR governance, commit atomicity, spec references, and path normalization. Fix the tag mismatches flagged above and these are ready.
.diffray/config.yaml (1)
1-54: Config is properly structured.File filters, review thresholds, and tag-based rule selection are correctly configured. The
tags.onlylist now covers all rule categories. Re-inclusion of workflow files ensures CI rules execute.
Update README.md to match actual config.yaml behavior: - Remove outdated .agents/sessions/** and .agents/retrospective/** from exclusions list - Add note explaining these directories are NOT excluded (intentional) - This allows naming convention rules (name_session_log_format, test_no_retrospective_without_validation) to execute The exclusions were intentionally removed in commit 38061ee to enable rule execution. README documentation now matches actual configuration. Addresses Copilot feedback on documentation/config mismatch. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Fix tag mismatches preventing rules from executing: git-discipline.yaml (git_commit_count_threshold): - Remove: git-history, autonomous-execution (not in allowlist) - Add: git, quality (matches allowlist) session-protocol.yaml (session_pr_template_required): - Remove: pr-level, documentation (not in allowlist) - Add: quality, consistency (matches allowlist) - Keep: traceability session-protocol.yaml (session_spec_reference_required): - Remove: pr-level, documentation (not in allowlist) - Add: quality, consistency (matches allowlist) - Keep: traceability All three rules now have tags matching config.yaml tags.only filter. Rules will execute properly despite having always_run: true. Addresses cursor[bot] feedback on tag filtering issues. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Fix workflow-constraints.yaml wf_error_handling rule: - Before: \b(?:gh|git|pwsh)\b\s|\bInvoke- - Inconsistent: first part requires space after command, second part doesn't - Could miss: commands at end of line, followed by punctuation - After: \b(?:gh|git|pwsh)\b|\bInvoke- - Consistent: both parts use only word boundaries - Catches: all command invocations regardless of trailing character Addresses Copilot feedback on pattern inconsistency. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
Fix powershell-patterns.yaml ps_error_handling_required example: - Before: gh @ghArgs 2>&1 - Incorrect: external commands require call operator with splatting - Would fail: PowerShell treats 'gh' as literal string - After: & gh @ghArgs 2>&1 - Correct: call operator & enables splatting with external commands - Works: properly invokes gh with splatted arguments The call operator is required when splatting arguments to external executables (non-cmdlet commands) in PowerShell. Addresses Copilot feedback on splatting syntax. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
…xample Fix security-patterns.yaml sec_command_injection_quoting example: - Before: & npx tsx @Args - Incorrect: @ splatting only works with cmdlets/functions, not external executables - Would fail: PowerShell doesn't expand @Args for external commands - After: & npx tsx $Args[0] $Args[1] $Args[2] - Correct: explicit array index expansion works with external commands - Alternative patterns: array expansion, Start-Process -ArgumentList PowerShell splatting (@) is a cmdlet/function feature. External executables require explicit argument passing via array indexing or other methods. Addresses Copilot feedback on PowerShell splatting limitations. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
| match: | ||
| file_glob: | ||
| - "**/*.psm1" | ||
| - "!**/*.Tests.ps1" |
There was a problem hiding this comment.
The rule description states "Every .psm1 module should have a corresponding .Tests.ps1 file with 80%+ coverage per ADR-006." However, the match pattern includes "!/*.Tests.ps1" which excludes test files from matching, but the file_glob only matches "/*.psm1". The exclusion pattern is unnecessary here since the glob already only matches .psm1 files. This exclusion has no effect and adds confusion.
| - "!**/*.Tests.ps1" |
| - "**/*.sh" | ||
| - "**/*.py" | ||
| - "!.githooks/**" # Git hooks are pre-existing exception | ||
| - "!.github/actions/**/*.sh" # External actions may have shell |
There was a problem hiding this comment.
The exclusion pattern "!.github/actions/**/*.sh" appears to be contradictory with the rule's purpose. If external actions may have shell scripts as stated in the comment, this exclusion is appropriate. However, consider documenting this exception in the project constraints or in a comment within the bad/good examples to explain why .github/actions shell scripts are allowed while others are prohibited.
| - "**/*.ps1" | ||
| - "**/*.psm1" | ||
| content_regex: | ||
| - "\\b(AI|LLM|GPT|Claude|Copilot)\\b" |
There was a problem hiding this comment.
The regex pattern "\b(AI|LLM|GPT|Claude|Copilot)\b" uses single backslash for word boundaries. This should be "\\b" in YAML strings for proper regex matching.
| - "\\b(AI|LLM|GPT|Claude|Copilot)\\b" | |
| - "\\\\b(AI|LLM|GPT|Claude|Copilot)\\\\b" |
| ├── config.yaml # Main configuration | ||
| ├── README.md # This file | ||
| └── rules/ | ||
| ├── powershell-patterns.yaml # PowerShell-specific rules (5 rules) |
There was a problem hiding this comment.
There's an inconsistency between the PR description which states "6 PowerShell-specific rules" and the actual content. The powershell-patterns.yaml file contains only 5 rules (ps_no_bash_python_scripts, ps_cmdletbinding_required, ps_error_handling_required, ps_avoid_write_host_in_modules, ps_proper_parameter_validation). The README correctly lists 5 rules, but the PR description table should be updated to match.
| filters: | ||
| useDefaults: true # Use built-in exclusions | ||
| exclude: | ||
| # AI memory (managed separately) |
There was a problem hiding this comment.
The comment "# AI memory (managed separately)" could be more precise. Consider clarifying that this refers to "Serena MCP memory files" or "AI agent memory storage" to better align with the project's terminology used elsewhere (e.g., .serena/memories/ as mentioned in the PR description).
| # AI memory (managed separately) | |
| # Serena MCP memory files / AI agent memory storage (managed separately) |
| file_glob: | ||
| - ".github/workflows/*.yml" | ||
| content_regex: | ||
| - "gh\\s+(pr|issue)\\s+list|for\\s+\\S+\\s+in" |
There was a problem hiding this comment.
The regex pattern "\b(gh|pr|issue)\s+list" has incorrect escaping. The word boundary "\b" should be "\\b" in YAML strings. Additionally, the pattern appears to have a logical issue: it matches "gh (pr|issue) list" but the alternation includes "gh" itself, which would match any occurrence of the word "gh" followed by whitespace and "list", which may not be the intended behavior. Consider revising to "\\bgh\\s+(pr|issue)\\s+list" for clarity.
| - "gh\\s+(pr|issue)\\s+list|for\\s+\\S+\\s+in" | |
| - "\\bgh\\s+(pr|issue)\\s+list|for\\s+\\S+\\s+in" |
| - "# nosec" | ||
| - "# skipcq" | ||
| - "# pragma: allowlist" | ||
|
|
There was a problem hiding this comment.
Security rule uses wrong comment syntax for JS/TS files
The sec_no_blind_suppression rule includes .js and .ts files in its file_glob but the content_regex patterns (# lgtm[, # nosec, # skipcq, # pragma: allowlist) only match #-style comments. JavaScript and TypeScript use // or /* */ for comments, not #. This means the rule would not detect security suppressions like // lgtm[js/...] in JS/TS files. Given the project is PowerShell-only per ADR-005, this has low practical impact, but the rule logic is inconsistent between the file types it claims to cover and the patterns it searches for.
Summary
Adds comprehensive diffray configuration for AI-assisted code review tailored to the ai-agents project. The configuration includes 40 project-specific rules organized into 10 categories, focusing on architectural and security review where AI adds the most value.
Specification References
.agents/and.serena/memories/Changes
Initial Configuration (22 rules)
.diffray/config.yamlwith model settings, filters, and tag-based rule selection.diffray/rules/powershell-patterns.yamlwith 6 PowerShell-specific rules.diffray/rules/workflow-constraints.yamlwith 5 GitHub Actions rules.diffray/rules/agent-templates.yamlwith 4 agent template protection rules.diffray/rules/session-protocol.yamlwith 7 governance/protocol rulesAdditional Rules from Project Analysis (18 rules)
.diffray/rules/security-patterns.yamlwith 4 security-specific rules.diffray/rules/git-discipline.yamlwith 3 git workflow rules.diffray/rules/user-facing-content.yamlwith 3 user content restriction rules.diffray/rules/testing-patterns.yamlwith 3 testing best practice rules.diffray/rules/naming-conventions.yamlwith 3 naming standard rules.diffray/rules/workflow-extended.yamlwith 2 extended workflow rulesDocumentation
.diffray/README.mdwith comprehensive configuration guide and usage documentationType of Change
Testing
Validation approach:
Agent Review
Security Review
Rationale: This PR adds code review configuration only. No authentication, secrets, or infrastructure execution changes.
Other Agent Reviews
Checklist
Configuration Summary
Total Rules: 40 across 10 categories
Base Settings
sonnet(balanced cost/quality).agents/sessions/,.agents/retrospective/,.serena/memories/)Key Design Decisions
Sources Referenced
Core governance:
PROJECT-CONSTRAINTS.md- Language, skill, workflow constraintsSESSION-PROTOCOL.md- Session start/end requirementsusage-mandatory.md- Skill usage requirementsSecurity patterns:
security-agent-vulnerability-detection-gaps.md- PowerShell security (CWE-22, CWE-77, CWE-78)powershell-security-ai-output.md- AI output validationsecurity-no-blind-suppression.md- No blind suppressionsQuality patterns:
git-hooks-fix-hook-errors-never-bypass.md- Git disciplineuser-facing-content-restrictions.md- User content rulestesting-002-test-first-development.md- Test patternsvalidation-anti-patterns.md- Validation patternsStandards:
naming-conventions.md- Artifact namingcode-style-conventions.md- Code standardsRelated ADRs
🤖 Generated with Claude Code