MPDX-9894 - Prepare MPDX agent files for 4 axes review system - #1948
MPDX-9894 - Prepare MPDX agent files for 4 axes review system#1948zweatshirt wants to merge 2 commits into
Conversation
Bundle sizes [mpdx-react]Compared against 0a0f7c3 No significant changes found |
zweatshirt
left a comment
There was a problem hiding this comment.
🔍 Multi-Agent Code Review — Verdict: ✅ CLEAN (0 blockers)
3 agents (Security, Standards, Architecture) · standard mode · Risk: 🟠 HIGH · auto-approval not triggered (review-process change → human review required)
This PR is correctly-scoped, contract-accurate prep for the 4-axis review model. Reviewed against its companion PR CruGlobal/claude-code-plugins#25, which ports the axis model into the production agent-review skill — the two are a designed pair.
The two PRs are a matched migration
| Contract element | Old skill (installed) | New skill (plugins#25) | This workflow change |
|---|---|---|---|
| Metadata | emits risk_score, no axes |
drops risk_score, emits axes:{danger,effort,…} |
reads axes.danger/axes.effort, drops risk_score ✅ |
| Gate key | risk_level |
risk_level (derived from danger) |
risk_level (unchanged) ✅ |
plugins#25 explicitly names this as the sanctioned path: "…or update the auto-approve workflow, which still reads risk_score." Removing risk_score here is required — the new skill stops emitting it.
Why deleting ## Special Pattern Detection is correct cleanup (not a weakening)
The new axis model subsumes every deleted modifier as a universal default, and handles the key cases more strictly:
yarn.lock/ lockfile drift → new model: "Any dependency-manifest edit or lockfile-only change → Sensitivity ≥ 2, Recoverability ≥ 1 (danger ≥ 3 → HIGH)" — caught universally, where the deleted repo rule gave only +1.- Dependency bumps (any package) → danger ≥ 3 → HIGH → not auto-approvable, making the old repo-specific "critical package" list redundant.
cache.tstypePolicies /next.configCSP → both are Critical-tier files, which the new model maps to Sensitivity ≥ 2 automatically.
The tier heading names are correctly preserved (the new model parses those exact strings), and the new prose "how each tier maps onto the risk axes is defined by the skill" is now accurate — plugins#25 implements exactly that mapping.
What's clean
- ✅ All four auto-approve gates (
risk_level,blockers,verdict, review-id dedup) untouched;risk_scorewas display-only. - ✅ No permission / secret / trigger-abuse regression in the workflow.
- ✅
axesparsing is null-safe (axes && typeof axes === 'object',danger ?? '?') — degrades gracefully to?/6on old-format metadata. - ✅ Debt reduced — the rules file no longer duplicates the skill's scoring weights; the skill is now the single source of truth.
Risk = HIGH (confirmed by the new rubric itself)
Both changed files match .claude/rules/code-review* and .github/workflows/*auto* — anchored at Sensitivity = 3 ("the review/CI process itself") in the very model this PR introduces. With Recoverability ~0–1, danger = 3–4 → HIGH/CRITICAL. The pipeline's own rubric agrees: a change to the review process warrants human review and is never auto-approvable.
The one actionable item (operational, not code)
🔵 Deployment sequencing. If this PR merges while reviewers still run the old plugin (v1.6.0), approval comments briefly render danger ?/6, effort ?/6 (null-safe, gates unaffected) and point-model scoring loses the now-redundant repo-specific modifiers. Both vanish once the cru-plugins/quality upgrade from plugins#25 is rolled out. Recommendation: land/deploy plugins#25 together with this PR (or first). No code change needed — the artifacts are correctly decoupled and degrade gracefully in every ordering.
Summary
| Agent | Blockers | Informational notes | Confidence |
|---|---|---|---|
| Security | 0 | 2 | High |
| Standards | 0 | 3 | High |
| Architecture | 0 | 2 | High |
Nothing to fix in the code. Only coordinate the merge/deploy with plugins#25.
🤖 Multi-agent review via /quality:agent-review (standard mode). Findings are informational; verdict CLEAN. Reply /dismiss: <reason> to any comment to acknowledge it.
| - `pages/api/Schema/**/*.{ts,graphql}` — REST-proxy resolvers, schemas, and data handlers (silent data-reshaping risk) | ||
| - `pages/api/Schema/**/datahandler*.ts` — response transformation feeding the REST proxy | ||
| - `pages/api/**/*.page.ts` (excluding `auth/` and `graphql-rest` — already Critical) — other API/lambda routes | ||
| - `src/components/**/*.graphql` — GraphQL operations (cache normalization and pagination correctness) |
There was a problem hiding this comment.
Are component GraphQL files actually high-risk?
|
I'm excited to try this out! |
Description
Must be merged with https://github.com/CruGlobal/claude-code-plugins/pull/25
The agent-review skill moved from a single 0–10 risk_score to a four-axis risk model (Blast Radius, Complexity, Sensitivity, Recoverability -> effort/danger). This PR updates the two places mpdx-react consumes that model so nothing shows stale data or reads the removed field.
Changes
Auto-approve workflow (ai-review-auto-approve.yml)
Code-review rules (.claude/rules/code-review.md)
Testing
N/A
Checklist:
/quality:agent-reviewcommand locally and fixed any relevant suggestions