review-test-cases

name	review-test-cases
description	This skill should be used when a quality review of TC-ID test case specifications is needed before any framework-specific implementation begins. It reviews the spec artifact only; it does not implement or rewrite tests. Triggers: "review test cases", "check test specs", "review TC-IDs", "audit test coverage", "are my test cases good", "validate test specs", "review docs/qa/test-cases/*.md", "check for gaps in test cases", "review before writing tests", and "quality check test specs". Inserted as the shared quality gate between `generate-test-cases` and execution-layer authoring — catches gaps, duplication, weak assertions, missing error paths, and invented scenarios before they get encoded into framework-specific code.
allowed-tools	Read Glob Grep Task

Review Test Cases

Review TC-ID test case specifications for completeness, accuracy, and quality before they are implemented in any execution framework. This is a quality gate — catch problems in the spec, not in the code.

Execution context: this review must run in a fresh subagent, not the agent that authored the spec. The orchestrator enforces this via the Task tool; the skill documents the contract so a drifting caller can see they're breaking it. Self-review reliably misses the gaps the author anchored on — an independent read-only context is the cheapest way to catch them.

Input

Parse the spec file path from: $ARGUMENTS

If no argument provided, search for docs/qa/test-cases/*.md files and review the most recently modified one.

Workflow

Step 1: Load the Spec and Context

1. Read the test case spec file
2. Read any related files for context:
   • docs/qa/feature-map.md if it exists
   • docs/qa/exploration-report.md if it exists
   • docs/qa/exploration/*.md if they exist
   • docs/qa/coverage-plan.md if it exists
3. Extract the target URL from the spec header

Step 2: Run the Review Checklist

Evaluate every test case against each criterion. Track findings by severity:

• BLOCKER — must fix before writing tests (missing critical paths, invented behavior, wrong URL)
• WARNING — should fix, will cause problems in implementation (vague steps, weak assertions, duplication)
• SUGGESTION — optional improvement (priority adjustment, better categorization, additional edge case)

2a. Coverage Completeness

Check	What to Look For
Happy path present	At least one Critical-priority test covers the primary success flow end-to-end
Error paths covered (when applicable)	For features that depend on backend retrieval/submission, expect coverage for meaningful server/network failures. For collection/data-driven UIs, expect empty-state coverage. For auth-gated features, expect session/auth edge cases. Missing an applicable category is a BLOCKER; a clearly documented non-applicable category is acceptable
Boundary conditions	Min/max values, empty inputs, special characters, long strings
Authentication edge cases	Session expiry, unauthorized access, role-based differences (if applicable)
Navigation edge cases	Back/forward, direct URL access, refresh mid-flow
Missing critical user actions	Every user-critical action in scope should appear in at least one test case. Ancillary controls may be omitted if they are not material to the target journey
TC-ID floor	Non-trivial features (more than two routes or more than one primary interactive surface per the exploration report) require ≥15 TCs. Fewer = BLOCKER "exploration too shallow — return to capture-feature-evidence"
Functional-to-visibility ratio	Count TCs that assert a state change (URL transition, data mutation, observable side effect, element value change after an action) vs TCs that assert render existence only. State-change TCs must be ≥60% of the total. Below 60% = BLOCKER "visibility coverage masquerading as functional coverage"
Deferred cap	Count TCs marked deferred / total. >20% = BLOCKER "scope too narrow, revisit exploration". Each deferred TC must carry blocker + un-defer plan + scope fields — missing fields = BLOCKER on that item
Inventory coverage	Compare TC count to the Element Inventory in docs/qa/exploration-report.md. If fewer than 50% of inventory rows have at least one TC that exercises them functionally, BLOCKER "coverage not proportional to observed surface area"
Mapped-feature grouping	If feature-map.md marks the feature MULTI-SURFACE, expect one spec per mapped sub-feature or an explicit justification for a combined file. A monolithic combined spec with no justification is a BLOCKER

2b. Specification Quality

Check	What to Look For
Steps are concrete	"Click the Submit button" not "submit the form"; "Enter '[e-mail]' in Email field" not "enter email"
Expected results are observable	Specific text, URL change, element state — not "page updates" or "works correctly"
Preconditions are explicit	Auth state, test data, feature flags, starting URL, viewport/browser assumptions, locale/timezone, and any execution-relevant environment note from exploration — nothing assumed
TC-IDs are sequential	No gaps, no duplicates, and use the canonical TC-<FEATURE>-<NNN> format
Priority is justified	Critical = blocks core journey; not everything is Critical
Categories are accurate	Happy Path vs Validation vs Edge Case — correctly classified
Execution-fragile controls are called out	Promoted TCs that rely on low-confidence selectors, viewport-specific layout, coordinate clicks, overlay interception, nth-match assumptions, or guessed DOM shape must carry a note or blocker; silence here is a WARNING at minimum

2c. Invented vs Observed

This is the most important check. Test cases should trace back to behavior that was actually observed or deliberately triggered during exploration, not assumed.

Red flags for invented scenarios:

• Specific error message text that wasn't observed (e.g., "Please enter a valid email" when the actual message might differ)
• Assumptions about validation rules without exploration evidence (e.g., "minimum 8 characters" without trying it)
• Test cases for UI elements that may not exist (e.g., "retry button" on error page without visiting the error page)
• Server-side behavior assumptions (e.g., "rate limit after 5 attempts" without evidence)

When suspicious: delegate a spot-check to a subagent with browser access (Task tool). Pass it the target URL, the specific TC-IDs under suspicion, and the claims to verify (element existence, error message text, validation behavior). Require structured evidence back, not prose — the same shape generate-test-cases uses:

Claim checked: <the TC-ID claim under suspicion>
Observed step: <what the subagent did>
URL: <current URL after action>
Observed controls:
  - Submit button: role=button, name=/submit/i
Observed result: <what actually happened — matched / differed, and how>

2d. Duplication and Overlap

• Flag test cases that test the same behavior with trivially different inputs
• Flag test cases where the steps are identical but expected results differ only cosmetically
• Merging candidates: cases that could be combined into a single parameterized test without losing coverage

2e. Implementability

• Flag steps that cannot be automated cleanly by downstream execution frameworks (for example, "verify email arrives" or "check database directly" without a stated automation path)
• Flag preconditions that require manual setup with no automation path
• Flag assertions that require visual comparison without specifying tolerance
• Flag test cases that depend on third-party services (payment processors, OAuth providers) without a mock strategy
• If the exploration artifact marked the relevant control Re-explore before automation = y and the spec promotes it anyway without a note, treat that as a BLOCKER

Step 3: Produce the Review Report

Output a structured review with this format:

# Test Case Review: <feature>

**Spec file:** <path>
**Total test cases:** <count>
**Review date:** <date>

## Verdict: PASS | PASS WITH WARNINGS | NEEDS REVISION

## Blockers (<count>)
- **TC-<ID>**: <issue description>

## Warnings (<count>)
- **TC-<ID>**: <issue description>

## Suggestions (<count>)
- **TC-<ID>**: <issue description>

## Coverage Gaps
- <Missing scenario that should be added>

## Duplication
- **TC-<ID>** and **TC-<ID>**: <overlap description>

## Summary
<2-3 sentences on overall spec quality and what to address before implementation>

Step 4: Verdict Rules

• PASS — no blockers, 2 or fewer warnings. Proceed to the execution-layer authoring skill.
• PASS WITH WARNINGS — no blockers, 3+ warnings. Can proceed but should address warnings.
• NEEDS REVISION — 1+ blockers. Do not proceed to framework-specific implementation until blockers are resolved.
• If promoted TCs are later deferred during execution and the spec changed materially as a result (TC removed, split, re-scoped, reprioritized, or execution notes/blockers changed in a way that affects implementability), route the updated spec back through this review before execution resumes. Do not treat that as a Gate 3-only fix.

Example: 0 blockers + 2 warnings = PASS. 0 blockers + 3 warnings = PASS WITH WARNINGS. 1+ blockers = NEEDS REVISION regardless of warning count.

Principles

• Fresh context — this review itself runs in a subagent that did not author the spec. Self-review misses the gaps the author anchored on. If the caller is the same agent that wrote the spec, they're using the skill wrong — the orchestrator should dispatch this via the Task tool with only the artifact path.
• Review-only — never modify the spec file; report findings for the author to act on
• Evidence over opinion — cite specific TC-IDs and quote specific steps/assertions when flagging issues
• Live-site spot-check — when specific claims look suspicious, delegate a bounded check to a second subagent with browser access to verify 2-3 claims. This is a sub-delegation for evidence; it does not replace the fresh-subagent-reviewer itself.
• Respect decomposition — when a feature map exists, review specs against the mapped sub-feature boundaries rather than treating the feature as one undifferentiated surface.
• Bounded output — the review report should be actionable and finite, not an exhaustive rewrite
• Severity matters — distinguish blockers from suggestions; not every imperfection is worth fixing before implementation
• Spec drift is real — if execution feedback materially changes the promoted TC set or its implementability assumptions, the spec has changed and must be reviewed again before signoff

Example Usage

/review-test-cases docs/qa/test-cases/login.md

/review-test-cases docs/qa/test-cases/checkout.md