name: Code quality review # Runs Claude against the PR diff, looking for code-quality issues the # linter, type-checker, and Mocha suite can't catch — divergence from the # CLI's messaging/spinner/error conventions (CLAUDE.md), duplicated # helpers, hand-rolled logic where a shared helper exists, dead code # left behind after refactors. # # Verdict shape mirrors security-review: # green → clean # yellow → advisory (style / minor) # red → high-confidence quality regression; blocks via branch protection # # Ported from root-platform's code-quality-review.yml, retuned for the # Workbench CLI. Uses Claude Max / Pro via OAuth token # (CLAUDE_CODE_OAUTH_TOKEN secret). on: pull_request: types: [opened, ready_for_review] issue_comment: types: [created] permissions: contents: read pull-requests: write issues: write statuses: write concurrency: group: code-quality-review-${{ github.event.pull_request.number || github.event.issue.number }} cancel-in-progress: true jobs: code-quality-review: # Auto-run once on PR open or draft->ready transition. Re-run on demand # when someone comments `/review` (all 4) or `/review quality` (just this one). # `startsWith(body, '/review')` is broad on purpose so trailing whitespace, # trailing newlines, and the checkbox-listener's `/review\n\n_Fired by # checkbox…_` body all match. if: | (github.event_name == 'pull_request' && github.event.pull_request.draft == false) || (github.event_name == 'issue_comment' && github.event.issue.pull_request != null && startsWith(github.event.comment.body, '/review') && !startsWith(github.event.comment.body, '/review security') && !startsWith(github.event.comment.body, '/review coverage') && !startsWith(github.event.comment.body, '/review breaking') && !startsWith(github.event.comment.body, '/review-help')) runs-on: ubuntu-latest timeout-minutes: 15 steps: - name: Resolve PR context id: ctx env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | # PR number + head SHA come from different event payloads. if [ "${{ github.event_name }}" = "pull_request" ]; then PR=${{ github.event.pull_request.number }} SHA=${{ github.event.pull_request.head.sha }} else PR=${{ github.event.issue.number }} SHA=$(gh api repos/${{ github.repository }}/pulls/$PR --jq .head.sha) fi echo "pr_number=$PR" >> "$GITHUB_OUTPUT" echo "head_sha=$SHA" >> "$GITHUB_OUTPUT" echo "Resolved PR=$PR SHA=$SHA" - name: React to trigger comment if: github.event_name == 'issue_comment' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | gh api -X POST \ repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}/reactions \ -f content=eyes >/dev/null || true - name: Checkout PR head uses: actions/checkout@v6 with: ref: ${{ steps.ctx.outputs.head_sha }} fetch-depth: 0 - name: Capture PR diff + relevant rules id: pr env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ steps.ctx.outputs.pr_number }} run: | gh pr diff "$PR_NUMBER" > /tmp/pr.diff diff_size=$(wc -c < /tmp/pr.diff) echo "diff_size=$diff_size" >> "$GITHUB_OUTPUT" echo "diff_lines=$(wc -l < /tmp/pr.diff)" >> "$GITHUB_OUTPUT" mkdir -p /tmp/ctx # Pre-stage the rule files that encode the CLI's canonical # conventions (messaging, spinners, errors, exit codes). The whole # point of this review is to flag deviation from these, so Claude # must read them before judging the diff. { for f in \ CLAUDE.md \ .cursor/rules/cli-messaging.mdc; do if [ -f "$f" ]; then echo "================================================================" echo "FILE: $f" echo "================================================================" cat "$f" echo fi done } > /tmp/ctx/rules.md echo '--- Diff preview (first 100 lines) ---' head -100 /tmp/pr.diff || true - name: Run Claude code-quality review id: review uses: anthropics/claude-code-base-action@v0.0.63 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} model: 'claude-opus-4-7' allowed_tools: 'Read,Grep,Bash' max_turns: '35' timeout_minutes: '12' prompt: | You are conducting a CODE-QUALITY review of a pull request to root-platform-cli — the Root Platform Workbench CLI (`rp`), a published npm package developers use to clone, push, publish, and test insurance product modules. Your job: find places the change diverges from the CLI's established conventions, duplicates already-existing helpers, hand-rolls logic where a canonical helper exists, or leaves dead code behind. False positives waste reviewer time but are recoverable. False negatives (this-pattern-was-already-wrong-and-now-spreads) compound silently. REPOSITORY: ${{ github.repository }} PR: #${{ github.event.pull_request.number }} — "${{ github.event.pull_request.title }}" DIFF SIZE: ${{ steps.pr.outputs.diff_size }} bytes, ${{ steps.pr.outputs.diff_lines }} lines ============================================================ !! CRITICAL: EMIT THE VERDICT JSON OR THE RUN IS WASTED ============================================================ Your run has a HARD 35-turn ceiling. If you do not emit the ::review_json:: block before that ceiling, the workflow falls back to a placeholder verdict and your compute is discarded. A partial / yellow verdict is ALWAYS better than no verdict. Plan your tool calls accordingly. ============================================================ OUTPUT FORMAT — READ THIS FIRST AND DO NOT FORGET IT ============================================================ Your FINAL assistant message MUST contain exactly one JSON object wrapped in the markers below, on lines by themselves with NO surrounding code fences (no ``` and no ~~~), and NO commentary after the closing marker. ::review_json:: { "verdict": "red" | "yellow" | "green", "summary": "", "findings": [ { "severity": "high" | "medium" | "low" | "info", "category": "messaging-convention-violation" | "canonical-helper-not-used" | "duplicated-helper" | "silent-failure" | "dead-code" | "unsafe-error-swallow" | "simplicity-regression", "location": "", "description": "", "fix": "" } ] } ::end:: Empty `findings: []` is fine. A yellow verdict with partial confidence is FAR better than no verdict at all. ============================================================ TURN BUDGET — HARD CAP ============================================================ Hard limit: 12 tool-calling turns before you MUST emit. - Turns 1-2: Read /tmp/pr.diff + /tmp/ctx/rules.md. - Turns 3-10: For each significantly changed source file, read it in full + grep for the canonical helper or pattern that should be used (src/helpers/). For new commands: check registration in src/index.ts, spinner usage, symbols usage, error handling. - Turn 11: STOP exploring. Turn 12: EMIT. ============================================================ You have Read, Grep, and Bash (git, rg, cat, head, tail) tools. The PR diff is in /tmp/pr.diff. The CLI's conventions are in /tmp/ctx/rules.md (CLAUDE.md + cli-messaging rules) — read them before judging. QUALITY CATEGORIES — scan for each: 1. **messaging-convention-violation** — the diff breaks the documented CLI output conventions: - async/network steps without runWithSpinner (or a manual createSpinner where runWithSpinner suffices); - hardcoded emoji/unicode instead of `symbols.*` from src/helpers/symbols.ts (breaks ASCII fallback on CI/legacy Windows); - wrong chalk semantics (green=success, blue=identifiers, yellow=warnings/commands, red=central handler only); - destructive commands without the yellow-warning + `ask.yesNo` `(y/n)? ` prompt + `--force` bypass, or wrong abort-message shape; - success lines not matching the past-tense, single-quoted-identifier format. 2. **canonical-helper-not-used** — the diff hand-rolls logic the CLI already encapsulates. Examples: raw ora usage instead of src/helpers/spinner.ts; raw fetch/https instead of RootAPIHelper (src/helpers/root-api.ts); ad-hoc file IO where fs-helpers / write-file helpers exist; ad-hoc readline instead of ask.yesNo. 3. **duplicated-helper** — a module-scope helper / validator / regex appears in MORE THAN ONE file under src/. Grep src/actions/ and src/helpers/ for identical or near- identical function definitions; extract to src/helpers/. 4. **silent-failure** — a failure path that exits 0 or prints an error and continues: `process.exit(0)` after an error message, catch blocks that log and return success, errors rendered with console.log instead of thrown as CLIError/PlatformError through actionErrorHandler. The CLI is used in CI pipelines — silent failures let CI publish stale artifacts. Flag as HIGH. 5. **dead-code** — exports / functions / files left behind after a refactor with no remaining callers. Grep before flagging. 6. **unsafe-error-swallow** — `catch { /* no-op */ }`, `.catch(() => null)`, or a dropped Promise rejection. Only telemetry (logUsage) may swallow errors. 7. **simplicity-regression** — the diff adds a flag / fallback / defensive check for a case that can't happen, or wraps existing behaviour in unnecessary indirection. SCOPING: - Only flag patterns introduced or worsened by this PR. Pre-existing code-style noise unrelated to the diff is NOT a finding (it's reviewer-fatigue fuel). - If a canonical pattern exists and the diff uses a less canonical one, that IS a finding — don't defer it as "stylistic". - Don't flag missing comments unless the WHY is genuinely non-obvious from the code. VERDICT — choose ONE based on the WORST severity in findings: - "red" — at least one HIGH-severity divergence (e.g. a silent-failure path, or a canonical-pattern bypass that hides errors). Blocks merge. - "yellow" — only MEDIUM, LOW, or INFO. Advisory; doesn't block. - "green" — clean alignment with the CLI conventions. SEVERITY GUIDELINES: - HIGH: a silent-failure path or a bypass that hides errors from users/CI. - MEDIUM: divergence from a documented convention with a clear canonical alternative. - LOW: stylistic cleanups (variable naming, ordering). - INFO: heads-up worth knowing (e.g. consider extracting this helper if it spreads). REMINDER — by turn 11 STOP, by turn 12 EMIT. Markers on their own lines, no code fences, no commentary after `::end::`. - name: Parse verdict, post comment, set status env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUMBER: ${{ steps.ctx.outputs.pr_number }} HEAD_SHA: ${{ steps.ctx.outputs.head_sha }} REPO: ${{ github.repository }} RUN_ID: ${{ github.run_id }} run: | python3 .github/scripts/post-claude-review.py \ --context "Code quality review" \ --review-kind "code-quality" \ --execution-file "${{ steps.review.outputs.execution_file }}" \ --workflow-file ".github/workflows/code-quality-review.yml" \ --findings-heading "Findings" - name: Delete trigger comment (issue_comment only) # When the workflow was fired by a /review comment, delete that # comment so the timeline doesn't fill up with trigger noise. # The verdict sticky comment is the persistent artifact. if: always() && github.event_name == 'issue_comment' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | gh api -X DELETE \ "repos/${{ github.repository }}/issues/comments/${{ github.event.comment.id }}" \ >/dev/null 2>&1 || true