Merge upstream origin/main (v2.60.0) with fork customizations preserved
Incorporates 78 upstream commits while preserving all local fork intent: - Keep deleted: dhh-rails, kieran-rails, dspy-ruby, andrew-kane-gem-writer (FastAPI pivot) - Merge both: ce-review (zip-agent-validator + design-conformance-reviewer wiring), kieran-python-reviewer (upstream pipeline + FastAPI conventions), ce-brainstorm/ce-plan/ce-work (upstream improvements + deploy wiring checks), todo-create (upstream template refs + assessment block), best-practices-researcher (upstream rename + FastAPI refs) - Accept remote: 142 remote-only files, plugin.json, README.md - Keep local: 71 local-only files (custom agents, skills, commands, voice) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -136,12 +136,14 @@ Routing rules:
|
||||
| `compound-engineering:review:kieran-typescript-reviewer` | TypeScript components, services, hooks, utilities, or shared types |
|
||||
| `compound-engineering:review:julik-frontend-races-reviewer` | Stimulus/Turbo controllers, DOM events, timers, animations, or async UI flows |
|
||||
|
||||
**CE conditional (migration-specific):**
|
||||
**CE conditional (migration & external review):**
|
||||
|
||||
| Agent | Select when diff includes migration files |
|
||||
|-------|------------------------------------------|
|
||||
| `compound-engineering:review:schema-drift-detector` | Cross-references schema.rb against included migrations |
|
||||
| `compound-engineering:review:deployment-verification-agent` | Produces deployment checklist with SQL verification queries |
|
||||
| Agent | Select when... |
|
||||
|-------|----------------|
|
||||
| `compound-engineering:review:design-conformance-reviewer` | Repo contains design documents or active plan matching current branch |
|
||||
| `compound-engineering:review:schema-drift-detector` | Diff includes migration files -- cross-references schema.rb against included migrations |
|
||||
| `compound-engineering:review:deployment-verification-agent` | Diff includes migration files -- produces deployment checklist with SQL verification queries |
|
||||
| `compound-engineering:review:zip-agent-validator` | PR URL contains `git.zoominfo.com` -- pressure-tests zip-agent comments for validity |
|
||||
|
||||
## Review Scope
|
||||
|
||||
@@ -342,7 +344,7 @@ Read the diff and file list from Stage 1. The 4 always-on personas and 2 CE alwa
|
||||
|
||||
Stack-specific personas are additive. A Rails UI change may warrant `kieran-rails` plus `julik-frontend-races`; a TypeScript API diff may warrant `kieran-typescript` plus `api-contract` and `reliability`.
|
||||
|
||||
For CE conditional agents, check if the diff includes files matching `db/migrate/*.rb`, `db/schema.rb`, or data backfill scripts.
|
||||
For CE conditional agents, check if the diff includes files matching `db/migrate/*.rb`, `db/schema.rb`, or data backfill scripts. If the PR URL contains `git.zoominfo.com`, select `zip-agent-validator`.
|
||||
|
||||
Announce the team before spawning:
|
||||
|
||||
@@ -380,7 +382,7 @@ Persona sub-agents do focused, scoped work and should use cheaper/faster models
|
||||
|
||||
Use the platform's cheapest capable model for all persona and CE sub-agents. In Claude Code, pass `model: "haiku"` in the Agent tool call. On other platforms, use the equivalent fast/cheap tier (e.g., `gpt-4o-mini` in Codex). If the platform has no model override mechanism or the available model names are unknown, omit the model parameter and let agents inherit the default -- a working review on the parent model is better than a broken dispatch from an unrecognized model name.
|
||||
|
||||
CE always-on agents (agent-native-reviewer, learnings-researcher) and CE conditional agents (schema-drift-detector, deployment-verification-agent) also use the cheaper model tier since they perform scoped, focused work.
|
||||
CE always-on agents (agent-native-reviewer, learnings-researcher) and CE conditional agents (design-conformance-reviewer, schema-drift-detector, deployment-verification-agent, zip-agent-validator) also use the cheaper model tier since they perform scoped, focused work.
|
||||
|
||||
The orchestrator (this skill) stays on the default model because it handles intent discovery, reviewer selection, finding merge/dedup, and synthesis -- tasks that benefit from stronger reasoning.
|
||||
|
||||
@@ -412,7 +414,7 @@ Each persona sub-agent returns JSON matching the findings schema included below:
|
||||
|
||||
**CE always-on agents** (agent-native-reviewer, learnings-researcher) are dispatched as standard Agent calls in parallel with the persona agents. Give them the same review context bundle the personas receive: entry mode, any PR metadata gathered in Stage 1, intent summary, review base branch name when known, `BASE:` marker, file list, diff, and `UNTRACKED:` scope notes. Do not invoke them with a generic "review this" prompt. Their output is unstructured and synthesized separately in Stage 6.
|
||||
|
||||
**CE conditional agents** (schema-drift-detector, deployment-verification-agent) are also dispatched as standard Agent calls when applicable. Pass the same review context bundle plus the applicability reason (for example, which migration files triggered the agent). For schema-drift-detector specifically, pass the resolved review base branch explicitly so it never assumes `main`. Their output is unstructured and must be preserved for Stage 6 synthesis just like the CE always-on agents.
|
||||
**CE conditional agents** (design-conformance-reviewer, schema-drift-detector, deployment-verification-agent, zip-agent-validator) are also dispatched as standard Agent calls when applicable. Pass the same review context bundle plus the applicability reason (for example, which migration files triggered the agent, which design docs were found, or that the PR URL matched `git.zoominfo.com`). For schema-drift-detector specifically, pass the resolved review base branch explicitly so it never assumes `main`. For zip-agent-validator, pass the full PR URL and the PR number so it can fetch comments from the GHE API. Their output is unstructured and must be preserved for Stage 6 synthesis just like the CE always-on agents.
|
||||
|
||||
### Stage 5: Merge findings
|
||||
|
||||
@@ -431,7 +433,7 @@ Convert multiple reviewer JSON payloads into one deduplicated, confidence-gated
|
||||
- report-only queue: `advisory` findings plus anything owned by `human` or `release`
|
||||
8. **Sort.** Order by severity (P0 first) -> confidence (descending) -> file path -> line number.
|
||||
9. **Collect coverage data.** Union residual_risks and testing_gaps across reviewers.
|
||||
10. **Preserve CE agent artifacts.** Keep the learnings, agent-native, schema-drift, and deployment-verification outputs alongside the merged finding set. Do not drop unstructured agent output just because it does not match the persona JSON schema.
|
||||
10. **Preserve CE agent artifacts.** Keep the learnings, agent-native, schema-drift, deployment-verification, and zip-agent-validator outputs alongside the merged finding set. Do not drop unstructured agent output just because it does not match the persona JSON schema. For zip-agent-validator specifically, its validated findings use the standard findings schema and enter the merge pipeline (steps 1-7) like persona findings. Its `residual_risks` entries (collapsed zip-agent comments) are preserved separately for the Zip Agent Validation section in Stage 6.
|
||||
|
||||
### Stage 6: Synthesize and present
|
||||
|
||||
@@ -450,8 +452,9 @@ Assemble the final report using **pipe-delimited markdown tables for findings**
|
||||
8. **Agent-Native Gaps.** Surface agent-native-reviewer results. Omit section if no gaps found.
|
||||
9. **Schema Drift Check.** If schema-drift-detector ran, summarize whether drift was found. If drift exists, list the unrelated schema objects and the required cleanup command. If clean, say so briefly.
|
||||
10. **Deployment Notes.** If deployment-verification-agent ran, surface the key Go/No-Go items: blocking pre-deploy checks, the most important verification queries, rollback caveats, and monitoring focus areas. Keep the checklist actionable rather than dropping it into Coverage.
|
||||
11. **Coverage.** Suppressed count, residual risks, testing gaps, failed/timed-out reviewers, and any intent uncertainty carried by non-interactive modes.
|
||||
12. **Verdict.** Ready to merge / Ready with fixes / Not ready. Fix order if applicable. When an `explicit` plan has unaddressed requirements, the verdict must reflect it — a PR that's code-clean but missing planned requirements is "Not ready" unless the omission is intentional. When an `inferred` plan has unaddressed requirements, note it in the verdict reasoning but do not block on it alone.
|
||||
11. **Zip Agent Validation.** If zip-agent-validator ran, summarize the results: how many zip-agent comments were evaluated, how many validated (these appear as findings in the severity-grouped tables above), and how many collapsed with reasons. This section provides traceability -- reviewers can see that zip-agent comments were evaluated, not ignored.
|
||||
12. **Coverage.** Suppressed count, residual risks, testing gaps, failed/timed-out reviewers, and any intent uncertainty carried by non-interactive modes.
|
||||
13. **Verdict.** Ready to merge / Ready with fixes / Not ready. Fix order if applicable. When an `explicit` plan has unaddressed requirements, the verdict must reflect it — a PR that's code-clean but missing planned requirements is "Not ready" unless the omission is intentional. When an `inferred` plan has unaddressed requirements, note it in the verdict reasoning but do not block on it alone.
|
||||
|
||||
Do not include time estimates.
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Persona Catalog
|
||||
|
||||
16 reviewer personas organized into always-on, cross-cutting conditional, and stack-specific conditional layers, plus CE-specific agents. The orchestrator uses this catalog to select which reviewers to spawn for each review.
|
||||
21 reviewer personas organized into always-on, cross-cutting conditional, stack-specific conditional, and language/framework conditional layers, plus CE-specific agents. The orchestrator uses this catalog to select which reviewers to spawn for each review.
|
||||
|
||||
## Always-on (4 personas + 2 CE agents)
|
||||
|
||||
@@ -48,19 +48,34 @@ These reviewers keep their original opinionated lens. They are additive with the
|
||||
| `kieran-typescript` | `compound-engineering:review:kieran-typescript-reviewer` | TypeScript components, services, hooks, utilities, or shared types |
|
||||
| `julik-frontend-races` | `compound-engineering:review:julik-frontend-races-reviewer` | Stimulus/Turbo controllers, DOM event wiring, timers, async UI flows, animations, or frontend state transitions with race potential |
|
||||
|
||||
## CE Conditional Agents (migration-specific)
|
||||
## Language & Framework Conditional (5 personas)
|
||||
|
||||
These CE-native agents provide specialized analysis beyond what the persona agents cover. Spawn them when the diff includes database migrations, schema.rb, or data backfills.
|
||||
Spawned when the orchestrator identifies language or framework-specific patterns in the diff. These provide deeper domain expertise than the general-purpose personas above.
|
||||
|
||||
| Agent | Focus |
|
||||
|-------|-------|
|
||||
| `compound-engineering:review:schema-drift-detector` | Cross-references schema.rb changes against included migrations to catch unrelated drift |
|
||||
| `compound-engineering:review:deployment-verification-agent` | Produces Go/No-Go deployment checklist with SQL verification queries and rollback procedures |
|
||||
| Persona | Agent | Select when diff touches... |
|
||||
|---------|-------|---------------------------|
|
||||
| `python-quality` | `compound-engineering:review:kieran-python-reviewer` | Python files, FastAPI routes, Pydantic models, async/await patterns, SQLAlchemy usage |
|
||||
| `fastapi-philosophy` | `compound-engineering:review:tiangolo-fastapi-reviewer` | FastAPI application code, dependency injection, response models, middleware, OpenAPI schemas |
|
||||
| `typescript-quality` | `compound-engineering:review:kieran-typescript-reviewer` | TypeScript files, React components, type definitions, generic patterns |
|
||||
| `frontend-races` | `compound-engineering:review:julik-frontend-races-reviewer` | Frontend JavaScript, Stimulus controllers, event listeners, async UI code, animations, DOM lifecycle |
|
||||
| `architecture` | `compound-engineering:review:architecture-strategist` | New services, module boundaries, dependency graphs, API layer changes, package structure |
|
||||
|
||||
## CE Conditional Agents (design, migration & external review)
|
||||
|
||||
These CE-native agents provide specialized analysis beyond what the persona agents cover. Spawn them when the diff includes database migrations, schema.rb, data backfills, design documents, or when the PR originates from specific platforms.
|
||||
|
||||
| Agent | Focus | Select when... |
|
||||
|-------|-------|----------------|
|
||||
| `compound-engineering:review:design-conformance-reviewer` | Surfaces deviations between the diff and the repo's design docs or implementation plan | The repo contains design documents (`docs/`, `docs/design/`, `docs/architecture/`, `docs/specs/`) or an active plan matching the current branch |
|
||||
| `compound-engineering:review:schema-drift-detector` | Cross-references schema.rb changes against included migrations to catch unrelated drift | The diff includes migration files or schema.rb |
|
||||
| `compound-engineering:review:deployment-verification-agent` | Produces Go/No-Go deployment checklist with SQL verification queries and rollback procedures | The diff includes migration files, schema.rb, or data backfills |
|
||||
| `compound-engineering:review:zip-agent-validator` | Pressure-tests zip-agent review comments for validity against full codebase context | The PR URL contains `git.zoominfo.com` |
|
||||
|
||||
## Selection rules
|
||||
|
||||
1. **Always spawn all 4 always-on personas** plus the 2 CE always-on agents.
|
||||
2. **For each cross-cutting conditional persona**, the orchestrator reads the diff and decides whether the persona's domain is relevant. This is a judgment call, not a keyword match.
|
||||
3. **For each stack-specific conditional persona**, use file types and changed patterns as a starting point, then decide whether the diff actually introduces meaningful work for that reviewer. Do not spawn language-specific reviewers just because one config or generated file happens to match the extension.
|
||||
4. **For CE conditional agents**, spawn when the diff includes migration files (`db/migrate/*.rb`, `db/schema.rb`) or data backfill scripts.
|
||||
5. **Announce the team** before spawning with a one-line justification per conditional reviewer selected.
|
||||
4. **For each language/framework conditional persona**, check whether the diff touches language or framework-specific patterns that warrant deeper domain expertise. These are additive with stack-specific personas, not replacements.
|
||||
5. **For CE conditional agents**, spawn when applicable: migration files (`db/migrate/*.rb`, `db/schema.rb`) or data backfill scripts trigger schema-drift-detector and deployment-verification-agent; design documents or active plans trigger design-conformance-reviewer; PR URLs containing `git.zoominfo.com` trigger zip-agent-validator.
|
||||
6. **Announce the team** before spawning with a one-line justification per conditional reviewer selected.
|
||||
|
||||
@@ -83,6 +83,15 @@ Use this **exact format** when presenting synthesized review findings. Findings
|
||||
- Residual risks: No rate limiting on export endpoint
|
||||
- Testing gaps: No test for concurrent export requests
|
||||
|
||||
### Zip Agent Validation
|
||||
|
||||
- Evaluated: 8 zip-agent comments
|
||||
- Validated: 2 (appear as findings #3 and #6 above)
|
||||
- Collapsed: 6
|
||||
- `app/services/order_service.rb:45`: "Missing error handling" -- handled by ApplicationService base class rescue
|
||||
- `app/controllers/api/orders_controller.rb:18`: "Unbounded query" -- pagination enforced by ApiController concern
|
||||
- _(4 more collapsed for stylistic/formatting concerns)_
|
||||
|
||||
---
|
||||
|
||||
> **Verdict:** Ready with fixes
|
||||
@@ -131,6 +140,7 @@ This fails because: no pipe-delimited tables, no severity-grouped `###` headers,
|
||||
- **Schema Drift Check section** -- results from schema-drift-detector. Omit if the agent did not run.
|
||||
- **Deployment Notes section** -- key checklist items from deployment-verification-agent. Omit if the agent did not run.
|
||||
- **Coverage section** -- suppressed count, residual risks, testing gaps, failed reviewers
|
||||
- **Zip Agent Validation section** -- summary of zip-agent comment evaluation: total, validated (with cross-references to findings table), collapsed (with reasons). Omit if the agent did not run.
|
||||
- **Summary uses blockquotes** for verdict, reasoning, and fix order
|
||||
- **Horizontal rule** (`---`) separates findings from verdict
|
||||
- **`###` headers** for each section -- never plain text headers
|
||||
|
||||
Reference in New Issue
Block a user