| 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
- Read the test case spec file
- Read any related files for context:
docs/qa/feature-map.md if it existsdocs/qa/exploration-report.md if it existsdocs/qa/exploration/*.md if they existdocs/qa/coverage-plan.md if it exists
- 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 'test@example.com' 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
