name: Test coverage review # Runs Claude against the PR diff, looking for test-coverage gaps the # existing Mocha suite and the developer's own additions miss — untested # command behaviour, weakened assertions, missing failure-path coverage. # # Verdict shape mirrors security-review: # green → adequately tested # yellow → advisory gap (worth a glance) # red → high-confidence silent-bug risk; blocks via branch protection # # Ported from root-platform's test-coverage-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: test-coverage-review-${{ github.event.pull_request.number || github.event.issue.number }} cancel-in-progress: true jobs: test-coverage-review: # Auto-run once on PR open or draft->ready transition. Re-run on demand # when someone comments `/review` (all 4) or `/review coverage` (just this one). 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 breaking') && !startsWith(github.event.comment.body, '/review quality') && !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 convention files (CLAUDE.md's Tests section is the # testing convention source of truth for this repo). { 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 test-coverage 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 TEST-COVERAGE review of a pull request to root-platform-cli — the Root Platform Workbench CLI (`rp`). Tests are Mocha + Sinon + Chai under src/**/__tests__/ (compiled and run over dist by `npm test`). Your job: find test-coverage gaps the existing suite and the developer's own additions miss. YOU HAVE TWO FAILURE MODES, NOT ONE. Weigh them equally: - FALSE NEGATIVE: a silent coverage hole (untested risky behaviour) ships to every developer machine and CI runner. - FALSE POSITIVE: you demand a test that adds no real protection — a test that wouldn't fail if the behaviour broke, or that pins trivial / logic-free code. This wastes the developer's time and trains people to ignore you. A missing test is only worth flagging if the test you'd ask for would plausibly catch a real bug. Coverage is a tool for finding untested RISK — it is NOT a target to maximise. Ask "would a reasonable engineer be worried this is untested?" — if not, stay silent. This is the SIBLING of the breaking-changes, code-quality, and security reviews — those answer "could this break / be ugly / be exploited?". You ask: "is the new behaviour adequately pinned by tests, and are existing tests pinning the right thing?" 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. The workflow parses these markers; without them the entire audit is wasted. ::review_json:: { "verdict": "red" | "yellow" | "green", "summary": "", "findings": [ { "severity": "high" | "medium" | "low" | "info", "category": "command-untested" | "failure-path-untested" | "exit-code-untested" | "messaging-contract-untested" | "boundary-untested" | "round-trip-gap" | "weakened-existing-test" | "test-shape-mismatch", "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 the verdict. Plan: - Turns 1-2: Read /tmp/pr.diff + /tmp/ctx/rules.md. - Turns 3-10: Sample 2-4 changed source files + their nearest test counterpart (sibling __tests__/ folder). Read the FULL test file, not just diffed lines — a removed assertion deep in the file is the highest-risk signal. For new/changed commands: check src/actions/__tests__/ for a test stubbing readAuthAndConfig + the helper modules. - Turn 11: STOP exploring no matter what. - Turn 12: EMIT the verdict. ============================================================ You have Read, Grep, and Bash (git, rg, cat, head, tail) tools. The PR diff is in /tmp/pr.diff. Testing conventions are in /tmp/ctx/rules.md (see the Tests section) — read them. COVERAGE CATEGORIES — scan for each: 1. **command-untested** — a new or behaviour-changed command in src/actions/ has no test in src/actions/__tests__/ exercising the changed behaviour (mocha/sinon sandbox, stubbed readAuthAndConfig + helpers). 2. **failure-path-untested** — new error handling (CLIError pre-checks, PlatformError mappings, catch/rethrow logic) with no test pinning the failure behaviour: error class, message content (identifier + Tip), and that side-effect stubs were NOT called past the failure point. 3. **exit-code-untested** — a path that must exit non-zero (thrown CLIError with ExitCodes) has no assertion on the exit code. The CLI runs in CI pipelines — wrong exit codes silently break deploys; this repo has been bitten before. 4. **messaging-contract-untested** — user-facing message changes (success lines, tips, prompts) with no assertion on the message content when the message is load-bearing (e.g. a Tip users act on, an abort line scripts grep for). Purely cosmetic wording changes don't need tests. 5. **boundary-untested** — boundary values (empty arrays, missing optional files, absent config fields, empty API responses) untested for new logic with natural boundaries. 6. **round-trip-gap** — changes to definition read/write (read-product-module-definition / write-product-module- definition and collection-module equivalents) without a round-trip assertion in the write-and-read test suites. These guard the on-disk format for every cloned workspace. 7. **weakened-existing-test** — the PR loosens an existing assertion (deleted .equals(), replaced narrow check with truthy, removed an it("...")). This is the "codify the bug as expected behaviour" failure mode. Tests should be ADDED to cover new cases, never WEAKENED to absorb them. 8. **test-shape-mismatch** — a test stubs so much that it asserts the stubs rather than the behaviour, or tests a helper through a command when a direct helper test is the right layer. Less critical but worth surfacing. PRIORITISE BY RISK — where to spend your attention: - FLAG (high value): push/publish/pull flows (they gate production deploys), exit codes, error handling, file read/write round-trips, API payload construction. A silent gap here breaks user CI or corrupts workspaces. - DO NOT FLAG (low value): trivial or logic-free code — pass-throughs, straight field mapping, type-only changes, constant wiring, spinner-text-only changes. Requesting tests here is a FALSE POSITIVE. - Quality over quantity: prefer ONE test that pins real behaviour over several that chase lines. If a test you'd ask for wouldn't fail when the behaviour breaks, don't ask for it. SCOPING: - Only flag gaps introduced or worsened by this PR. Pre-existing gaps unrelated to the diff are noise. - Trivial changes (doc-only, dep bump in package-lock.json, comment-only diffs) → green, empty findings. - If the repo's broader testing convention genuinely doesn't cover a surface, surface that as info — but don't expect the PR to fix the whole codebase. VERDICT — choose ONE. "green" is a real, expected outcome: reach for it whenever the change is adequately tested. You do NOT need zero possible improvements to approve. - "green" — APPROVE. The risk-important behaviour introduced or changed by this PR (deploy-gating flows, exit codes, error handling, file round-trips) is tested well enough that a reasonable engineer would be confident merging. Most well-made PRs should land here. - "yellow" — advisory only. A genuine MEDIUM gap on risk-important behaviour is worth mentioning, but the happy path is covered and it doesn't block. Do NOT go yellow just to have something to say. - "red" — at least one HIGH severity gap (likely silent production/CI bug). Blocks merge. Reserve for real, demonstrable risk — not "could add more tests". SEVERITY GUIDELINES: - HIGH: a new command / failure path / exit-code contract has ZERO test coverage; OR an existing assertion was deleted that pinned now-broken behaviour. - MEDIUM: a boundary / round-trip case is missing but the happy path is covered. - LOW: minor coverage gap (extra branch in a helper). - INFO: heads-up worth knowing. REMINDER — by turn 11 STOP, by turn 12 EMIT. A yellow verdict with partial confidence is FAR better than no verdict at all. - 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 "Test coverage review" \ --review-kind "test-coverage" \ --execution-file "${{ steps.review.outputs.execution_file }}" \ --workflow-file ".github/workflows/test-coverage-review.yml" \ --findings-heading "Coverage gaps" - 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