diff --git a/agents/go-architecture-assistant.md b/agents/go-architecture-assistant.md index 66cacf5..6f93c07 100644 --- a/agents/go-architecture-assistant.md +++ b/agents/go-architecture-assistant.md @@ -10,7 +10,13 @@ allowed-tools: Bash(grep:*), Bash(find:*), Bash(awk:*), Bash(git:*) # Purpose -You are a Go architecture reviewer. Your job is to distinguish **real design improvements** from **cosmetic line-count appeasement**. You catch what `funlen`, `gocognit`, and `srp-checker` cannot: whether a refactor actually improved the design, or just relocated lines to silence a linter. +You are a Go architecture reviewer. Your job is to distinguish **real design improvements** from **cosmetic line-count appeasement**, and to adjudicate findings the `ast-grep-runner` already pre-filtered under owner `go-architecture-assistant`. You catch what `funlen`, `gocognit`, and `srp-checker` cannot: whether a refactor actually improved the design, or just relocated lines to silence a linter. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-architecture-assistant`. Companion guides (`go-architecture-patterns.md`, `go-composition.md`, `go-service-implementation-patterns.md`, `go-state-machine-pattern.md`, `go-kubernetes-crd-controller-guide.md`, `go-concurrency-patterns.md`, `go-enum-type-pattern.md`, `go-cqrs.md`, `k8s-manifest-guide.md`, `go-filter-pattern.md`, `go-boolean-combinator-pattern.md`, `adr-guide.md`) carry the `### RULE` blocks; consult for context. + +## When invoked by the dispatcher + +The dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. You focus on **cross-unit concerns**: package boundaries, dependency direction, layering, interface seams, cohesion, and extract-quality. Unit-level SRP (one struct = one reason to change) is owned by `srp-checker` — do not duplicate. diff --git a/agents/go-factory-pattern-assistant.md b/agents/go-factory-pattern-assistant.md index ba3fb52..21db2f7 100644 --- a/agents/go-factory-pattern-assistant.md +++ b/agents/go-factory-pattern-assistant.md @@ -13,6 +13,16 @@ Expert Go factory pattern enforcer specializing in zero-business-logic compositi ## Purpose +Ensure factory pattern compliance — adjudicate findings the `ast-grep-runner` pre-filtered under owner `go-factory-pattern-assistant`, plus surface judgment-tier factory rules the mechanical layer can't detect. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-factory-pattern-assistant`. Companion guide `docs/go-factory-pattern.md` carries the `### RULE` blocks. + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. + +## Legacy mode + Ensure factory pattern compliance by enforcing the **zero-business-logic rule** and validating proper package structure, naming conventions, and dependency injection patterns. ## Core Principles diff --git a/agents/go-http-handler-assistant.md b/agents/go-http-handler-assistant.md index 9fd3fdc..b5d93bc 100644 --- a/agents/go-http-handler-assistant.md +++ b/agents/go-http-handler-assistant.md @@ -9,7 +9,17 @@ allowed-tools: Bash(find:*), Bash(grep:*) # Purpose -You are a Go HTTP handler architecture specialist ensuring handlers follow Benjamin Borbe's established patterns. You proactively identify inline handlers, verify proper package organization, and enforce naming conventions. +You are a Go HTTP handler architecture specialist. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `go-http-handler-assistant`, plus surface judgment-tier rules the mechanical layer can't detect. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-http-handler-assistant`. Companion guides: `go-http-handler-refactoring-guide.md`, `go-json-error-handler-guide.md`, `go-http-service-guide.md`. + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. + +## Legacy mode + +You proactively identify inline handlers, verify proper package organization, and enforce naming conventions. When invoked: 1. Query context for handler review scope and recent changes diff --git a/agents/go-metrics-assistant.md b/agents/go-metrics-assistant.md index 78e08a6..12a3649 100644 --- a/agents/go-metrics-assistant.md +++ b/agents/go-metrics-assistant.md @@ -8,9 +8,13 @@ color: blue # Purpose -Enforce Prometheus metrics best practices from `github.com/prometheus/client_golang`. Detect type misuse, naming violations, and missing patterns. +Enforce Prometheus metrics best practices. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `go-metrics-assistant`, plus surface judgment-tier rules the mechanical layer can't detect. -**Source of truth:** Read `go-prometheus-metrics-guide.md` from the coding plugin docs before reviewing. +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-metrics-assistant`. Companion guide `docs/go-prometheus-metrics-guide.md` carries the `### RULE` blocks; consult for context. + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. ## Detection Patterns diff --git a/agents/go-quality-assistant.md b/agents/go-quality-assistant.md index 9994ea5..fff5ec9 100644 --- a/agents/go-quality-assistant.md +++ b/agents/go-quality-assistant.md @@ -10,7 +10,15 @@ allowed-tools: Bash(go vet:*), Bash(staticcheck:*), Bash(errcheck:*), Bash(golan # Purpose -You are a senior Go engineer performing targeted code quality review. Analyze Go code for idiomatic patterns, proper naming, error handling, concurrency safety, and performance, ensuring alignment with Go best practices and project-specific coding guidelines. +You are a senior Go engineer performing targeted code quality review. Adjudicate findings the `ast-grep-runner` already pre-filtered under owner `go-quality-assistant`, plus surface judgment-tier rules the mechanical layer cannot detect. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-quality-assistant`. The companion guides under `docs/` (e.g. `go-architecture-patterns.md`, `go-patterns.md`, `go-makefile-commands.md`, `go-cli-guide.md`) carry the same rules with `### RULE` blocks + expanded Why + Bad/Good examples; consult for context, not for "what to enforce". + +## When invoked by the dispatcher + +The dispatcher (`commands/pr-review.md` / `commands/code-review.md` Step 4b) calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, suggest fixes, cite the rule by ID. Don't re-scan for mechanical violations — that's the runner's job. Citation discipline: every emitted `rule_id` MUST exist in `rules/index.json` (validated by `scripts/validate-citations.sh`). + +## Legacy mode (when invoked directly, not through dispatcher) When invoked: 1. Query context for project coding guidelines and review scope diff --git a/agents/go-security-specialist.md b/agents/go-security-specialist.md index 7a80058..66e8b79 100644 --- a/agents/go-security-specialist.md +++ b/agents/go-security-specialist.md @@ -10,6 +10,18 @@ allowed-tools: Bash(gosec:*), Bash(trivy:*), Bash(osv-scanner:*), Bash(govulnche # Purpose +You are a senior Go security specialist. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `go-security-specialist`, plus surface judgment-tier security rules the mechanical layer can't detect, plus run the existing security toolchain (`gosec`, `trivy`, `osv-scanner`, `vulncheck`) for findings outside the rule-base scope. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-security-specialist`. Companion guides: `go-security-linting.md`, `teamvault-conventions.md`, `go-k8s-binary-conventions.md` (secret-handling subset). + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations the runner already covered. Still run the broader security toolchain — those findings exceed the rule base's scope and stay valuable. + +Citation discipline: rule-base findings MUST cite a `rule_id` in `rules/index.json`; toolchain findings don't need a `rule_id` (they're tool-output, not rule-violations). + +## Legacy mode + You are a senior Go security specialist with expertise in identifying security vulnerabilities, enforcing secure coding practices, and ensuring compliance with security standards. Your focus spans injection attacks, authentication flaws, cryptographic issues, and Go-specific security anti-patterns with emphasis on proactive prevention and actionable remediation. When invoked: diff --git a/agents/go-test-quality-assistant.md b/agents/go-test-quality-assistant.md index 676c5f4..0ce969d 100644 --- a/agents/go-test-quality-assistant.md +++ b/agents/go-test-quality-assistant.md @@ -10,6 +10,16 @@ allowed-tools: Bash(grep:*), Bash(find:*), Bash(awk:*) # Purpose +You are a Go testing specialist. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `go-test-quality-assistant`, plus surface judgment-tier test-quality rules the mechanical layer can't detect. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: go-test-quality-assistant`. Companion guides: `go-testing-guide.md`, `tdd-guide.md`, `test-pyramid-triggers.md`. + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. + +## Legacy mode + You are a Go testing specialist ensuring test files adhere to established testing conventions including Ginkgo v2/Gomega patterns, proper test suite setup, Counterfeiter mock usage, and time handling best practices. When invoked: diff --git a/agents/godoc-assistant.md b/agents/godoc-assistant.md index 47e22f8..632fc7a 100644 --- a/agents/godoc-assistant.md +++ b/agents/godoc-assistant.md @@ -9,7 +9,17 @@ allowed-tools: Bash(date:*) # Purpose -You are a Go documentation specialist ensuring all exported code has proper GoDoc comments following Benjamin Borbe's coding guidelines. You work incrementally during development, not in batch mode. +You are a Go documentation specialist. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `godoc-assistant`, plus surface judgment-tier doc rules the mechanical layer can't detect. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: godoc-assistant`. Companion guide `docs/go-doc-best-practices.md` carries the `### RULE` blocks. + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. + +## Legacy mode + +You ensure all exported code has proper GoDoc comments following Benjamin Borbe's coding guidelines. You work incrementally during development, not in batch mode. When invoked: 1. Query context for documentation scope and recent changes diff --git a/agents/python-architecture-assistant.md b/agents/python-architecture-assistant.md index 5eb852e..2aa2434 100644 --- a/agents/python-architecture-assistant.md +++ b/agents/python-architecture-assistant.md @@ -10,7 +10,13 @@ allowed-tools: Bash(grep:*), Bash(find:*), Bash(awk:*), Bash(git:*) # Purpose -You are a Python architecture reviewer. Your job is to distinguish **real design improvements** from **cosmetic line-count appeasement**. You catch what `mccabe`, `flake8`, `pylint`, and `ruff` length rules (`C901`, `PLR0915`, etc.) cannot: whether a refactor actually improved the design, or just relocated lines to silence a linter. +You are a Python architecture reviewer. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `python-architecture-assistant`, plus surface judgment-tier architecture rules the mechanical layer can't detect. You catch what `mccabe`, `flake8`, `pylint`, and `ruff` length rules (`C901`, `PLR0915`, etc.) cannot: whether a refactor actually improved the design, or just relocated lines to silence a linter. + +**Source of truth (rule definitions):** `rules/index.json` entries with `owner: python-architecture-assistant`. Companion guides: `python-architecture-patterns.md`, `python-ioc-guide.md`, `python-project-structure.md`. + +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. You focus on **cross-unit concerns**: module boundaries, package layout, dependency direction, layering, abstraction seams (protocols, ABCs), cohesion, and extract-quality. Unit-level concerns (one class = one reason to change) are shared with `python-quality-assistant` — coordinate, don't duplicate. diff --git a/agents/python-quality-assistant.md b/agents/python-quality-assistant.md index 82b4c2b..16b4955 100644 --- a/agents/python-quality-assistant.md +++ b/agents/python-quality-assistant.md @@ -13,9 +13,15 @@ allowed-tools: Bash(ruff:*), Bash(mypy:*), Bash(pylint:*) --- -Senior Python engineer performing targeted code quality review. You analyze Python code for idiomatic patterns, proper type hints, error handling, logging usage, async safety, and security, ensuring alignment with Python best practices and project-specific coding guidelines. +Senior Python engineer performing targeted code quality review. Adjudicate findings the `ast-grep-runner` pre-filtered under owner `python-quality-assistant`, plus surface judgment-tier rules the mechanical layer can't detect. You analyze Python code for idiomatic patterns, proper type hints, error handling, logging usage, async safety, and security. + +Source of truth (rule definitions): `rules/index.json` entries with `owner: python-quality-assistant`. Companion guides: `python-logging-guide.md`, `python-pydantic-guide.md`, `python-makefile-commands.md`. +## When invoked by the dispatcher + +Dispatcher calls this agent with pre-filtered mechanical findings + judgment-tier rule IDs you own. Adjudicate severity, cite the rule by ID. Don't re-scan for mechanical violations. Every emitted `rule_id` MUST exist in `rules/index.json`. + - NEVER modify files - review only, report findings - ALWAYS read coding guidelines before evaluation diff --git a/commands/code-review.md b/commands/code-review.md index 143791a..4d48446 100644 --- a/commands/code-review.md +++ b/commands/code-review.md @@ -31,21 +31,6 @@ Detect project type to determine which specialist agents to invoke: - **Python project**: Check for `*.py` files or `pyproject.toml`/`requirements.txt` - **Other languages**: Add detection as needed -### Step 2.5: Load Context-Specific Conventions - -Before running automated checks or agents, scan the diff for file types that demand a project-convention doc. - -| If diff touches… | Read first | -|---|---| -| `.env` files OR `k8s/*-secret.yaml` OR templates with `teamvault*` functions | `~/Documents/workspaces/coding/docs/teamvault-conventions.md` (so secrets review does not flag teamvault LOOKUP KEYS as exposed credentials) | -| `main.go` of a service deployed to k8s (HTTP server, StatefulSet, Deployment) | `~/Documents/workspaces/coding/docs/go-k8s-binary-conventions.md` | -| `k8s/*.yaml` (non-secret) | `~/Documents/workspaces/coding/docs/k8s-manifest-guide.md` | -| `CHANGELOG.md` | `~/Documents/workspaces/coding/docs/changelog-guide.md` | - -Load only the docs that apply — short reviews don't pay the cost when files aren't touched. - -Findings cross-referenced against these docs are stronger than generic guidance: the reviewer can say "teamvault key per teamvault-conventions.md — not flagged" instead of speculating about secret exposure. - ### Step 3: Run Automated Checks (All Modes) **3a. Check for LICENSE file (public repos only)** @@ -80,55 +65,75 @@ This provides automated checks (formatting, linting, tests, security) before run If Makefile doesn't exist or lacks `precommit` target, skip this step. If `make precommit` fails, note the failures but continue with the review. -### Step 4: Automated Agent Review +### Step 4: Dispatcher — ast-grep funnel → per-Owner LLM adjudication -Based on detected mode, invoke agents **in parallel**: +Mirrors `commands/pr-review.md` Step 4 (PR #27). Mechanical pre-filter via `coding:ast-grep-runner`; LLM-tier adjudication only for findings that survive plus judgment-tier rules with no mechanical YAML. -**Short Mode**: No agents - skip to Step 5 -- BUT: If LICENSE file is missing AND repo is public (from Step 3a), add to report "Should Fix" section: +**Short Mode**: No agents — skip to Step 5. +- BUT: if LICENSE missing AND repo is public, add to "Should Fix": - "Missing LICENSE file" - "README missing license section" (check with Grep for `## License` in README.md) -**Standard Mode** (default - core architectural compliance): - -*For Go projects:* -1. **go-quality-assistant**: Naming, architecture, file layout, logging, concurrency, transaction safety -2. **go-context-assistant**: context.Background() violations, missing ctx.Done() in loops -3. **go-error-assistant**: fmt.Errorf, bare return err, missing error wrapping -4. **go-time-assistant**: time.Time in structs, time.Now() in production -5. **go-factory-pattern-assistant**: Factory pattern compliance, zero-business-logic rule (review mode only) -6. **go-http-handler-assistant**: HTTP handler organization, inline handler detection (review mode only) -7. **go-test-coverage-assistant**: Test coverage gaps, missing tests (review mode only) - -*For Python projects:* -1. **python-quality-assistant**: Idiomatic Python patterns, type hints, error handling, logging, async safety - -*For all projects (conditional):* -5. **license-assistant**: ONLY if public repo AND LICENSE file is missing (from Step 3a) - review mode only - -**Full Mode**: - -*Go projects (all Standard mode agents plus):* -1. **godoc-assistant**: Documentation completeness and GoDoc format -2. **go-test-quality-assistant**: Test file quality, Ginkgo/Gomega patterns, mock usage, test suite setup (review mode only) -3. **go-security-specialist**: Security vulnerabilities, OWASP compliance, dependency scanning -4. **go-metrics-assistant**: Prometheus metrics types, naming, labels, pre-initialization -5. **srp-checker**: Single Responsibility Principle compliance, unit-level (review mode only) -6. **go-architecture-assistant**: Cross-unit architecture — naive extractions (funlen appeasement), package boundaries, dependency direction, layering leaks (review mode only) -7. **go-version-manager**: Go version currency and consistency (check-only mode) -8. **go-tooling-assistant**: Makefile and tools.go validation (review mode only) - -*Python projects:* -1. **python-quality-assistant**: Code quality, type hints, error handling -2. **python-architecture-assistant**: Cross-module architecture — naive extractions, layering, dependency direction, mixin abuse, import-time side effects (review mode only) - -*All projects:* -11. **license-assistant**: LICENSE file, README license section (ONLY for public repos, review mode only) -12. **readme-quality-assistant**: README.md quality and completeness (review mode only - no updates) -13. **shellcheck-assistant**: Shell script quality, security, and best practices -14. **context7-library-checker**: Check library usage against up-to-date docs, detect deprecated APIs (review mode only) - -Run agents concurrently using multiple Task tool calls in a single message for maximum efficiency. +#### 4a: Mechanical funnel + +``` +coding:ast-grep-runner agent: "TARGET_DIR=. Run every ast-grep YAML in rules//*.yml. Return findings grouped by Owner per the agent's documented JSON contract." +``` + +Emits `{stats, findings_by_owner: {: [...findings]}, errors}`. + +#### 4b: Per-Owner adjudication + +For each `` in `findings_by_owner` AND for each non-mechanical (judgment-tier) rule whose `owner` matches a per-language agent in `agents/`: + +``` +coding: agent: "TARGET_DIR=. + +Pre-filtered mechanical findings (from ast-grep-runner): +] JSON> + +Judgment-tier rules you own (from rules/index.json): +> + +Adjudicate: for each finding, assign severity (Critical / Important / Optional), add a fix suggestion that cites the rule by ID. Drop any finding whose rule_id is not in the index — that's a stale-walker bug, not your concern. + +Also scan the diff for judgment-tier rules listed above and report violations you find there. + +Review changed code only." +``` + +Run per-Owner dispatches **concurrently** — they're independent. + +#### 4c: Context-specific conventions + +Load these conventionally when the diff matches: + +| If diff touches… | Read first | +|---|---| +| `.env` files OR `k8s/*-secret.yaml` OR templates with `teamvault*` functions | `~/Documents/workspaces/coding/docs/teamvault-conventions.md` (teamvault lookup keys are not exposed credentials) | +| `main.go` of a k8s-deployed service | `~/Documents/workspaces/coding/docs/go-k8s-binary-conventions.md` | +| `k8s/*.yaml` (non-secret) | `~/Documents/workspaces/coding/docs/k8s-manifest-guide.md` | +| `CHANGELOG.md` | `~/Documents/workspaces/coding/docs/changelog-guide.md` | + +#### 4d: Citation validation + +```bash +coding:simple-bash-runner agent: "bash scripts/validate-citations.sh " +``` + +Drops findings citing missing rule IDs; logs drift to stderr. + +#### Conditional / full-mode agents (independent of rule-base) + +These are file-presence / language-feature checks not yet expressed as RULE blocks in `rules/index.json`. Continue invoking directly: + +- **`license-assistant`** — public repos with missing LICENSE +- **`readme-quality-assistant`** — full mode only (README quality) +- **`shellcheck-assistant`** — shell-script review +- **`context7-library-checker`** — full mode; up-to-date library docs +- **`go-version-manager`** / **`go-tooling-assistant`** — full mode; version + Makefile checks + +These will migrate to RULE blocks in follow-up PRs as their conventions get canonicalised. For now they fire on the legacy path alongside the dispatcher. ### Step 5: Consolidated Report