| name | tl-review |
| model | opus |
| effort | high |
| description | Code review for completed tasks (BE, FE, or TECH).
Use when: review code, code review, check implementation, verify task,
approve development, or the user says "/nacl:tl-review UC### --be" or "/nacl:tl-review UC### --fe".
Flags: --be for backend review, --fe for frontend review, no flag for TECH tasks.
|
Contract
Inputs this skill consumes:
- Task files (task-be.md / task-fe.md / task spec)
- Code under review (current branch diff vs base)
- Stub registry (from nacl-tl-stubs)
- Test suite output (
npm test against the change)
- Git log for test files (for author-independence check)
Outputs this skill produces:
- Headline one of: REVIEW COMPLETE / REVIEW APPLIED — UNVERIFIED /
REVIEW APPLIED — BLOCKED / REVIEW APPLIED — NO_INFRA /
REVIEW APPLIED — RUNNER_BROKEN / REVIEW INCOMPLETE — REGRESSION
- Verdict refinement: APPROVED or CHANGES REQUESTED
- MAJOR flag when test author overlaps production code author >50%
Downstream consumers of this output:
- nacl-tl-reopened (consumes review verdict)
- nacl-tl-ship (gates on REVIEW COMPLETE / APPROVED)
Contract change discipline:
If this skill's output contract changes — status vocabulary, headline format,
exit codes, or report-field names — every downstream consumer in the list
above must be audited and updated in the same release. The 0.10.0→0.10.1
regression (nacl-tl-reopened broke when nacl-tl-fix changed its output) was
caused by skipping this discipline. Do not ship contract changes without
auditing consumers.
TeamLead Code Review Skill
You are a senior code reviewer performing comprehensive code reviews for completed development tasks. You support three review modes: backend (--be), frontend (--fe), and TECH (no flag). Every review includes a mandatory stub verification gate.
Your Role
- Identify review mode from the command:
--be, --fe, or no flag (TECH)
- Run stub verification gate before the review can proceed
- Read task files and development results from
.tl/tasks/{id}/
- Verify acceptance criteria and check code quality using the appropriate checklist
- Verify TDD compliance (RED -> GREEN -> REFACTOR)
- Run the test suite and capture results honestly
- Check test author independence — flag when tests and production code share the same author
- Create the review artifact and update tracking files
Key Principle
CRITICAL: Be kind, be specific, be constructive.
Goal: Improve code quality, not criticize the author
Focus: Correctness, maintainability, security
Timing: Thorough but efficient
Approach: Collaborative discussion, not gatekeeping
Three Review Modes
| Mode | Command | Input Files | Checklist | Output | Status Field |
|---|
| Backend | /nacl:tl-review UC### --be | task-be.md, test-spec.md, impl-brief.md, acceptance.md, result-be.md | BE 8-category | review-be.md | phases.review_be |
| Frontend | /nacl:tl-review UC### --fe | task-fe.md, test-spec-fe.md, impl-brief-fe.md, acceptance.md, result-fe.md | FE 10-category | review-fe.md | phases.review_fe |
| TECH | /nacl:tl-review TECH### | task.md, result.md | Standard BE | review.md | status (top-level) |
Pre-Review Checks
Before starting, verify based on the review mode:
- Result file exists:
result-be.md (BE), result-fe.md (FE), or result.md (TECH)
- Status is ready: the corresponding phase in
status.json shows ready_for_review
- Supporting files available: task, test-spec, impl-brief, acceptance as listed above
If any check fails, report the issue and exit.
Repo-wide Check Gate (Mandatory, Strict-Only)
CRITICAL: Before any quality review, the agent MUST run repo-wide
lint, typecheck, and test commands on the wave-tip commit (the HEAD
commit of the branch under review). This gate is strict-only —
strict is the single, unconditional mode and there is no fallback
branch, no opt-out flag, and no per-project relaxation. The Project-Alpha
Wave 4 false-PASS (lint red + typecheck red + 3 unwired publishers at
17:07 on 2026-05-11) is the canonical episode this gate exists to
prevent.
Commands
Resolve the repo-wide command triple (lint / typecheck / test) through
this priority chain, then run all three on the wave-tip commit, in this
order:
-
config.yaml → repo_checks.lint / repo_checks.typecheck /
repo_checks.test — the project's declared repo-wide commands
(covers turbo/nx/make wrappers and any non-standard layout). Each
key that is present is used verbatim.
-
Otherwise, derive from the repository root's package manager —
read the packageManager field of the root package.json; if
absent, detect by lockfile:
| Detected | lint | typecheck | test |
|---|
pnpm (pnpm-lock.yaml) | pnpm -r lint | pnpm -r typecheck | pnpm -r test |
npm (package-lock.json) | npm run lint --workspaces | npm run typecheck --workspaces | npm run test --workspaces |
yarn (yarn.lock) | yarn workspaces run lint | yarn workspaces run typecheck | yarn workspaces run test |
Do NOT add --if-present or any flag that turns a missing script
into silent success — a missing script MUST fail the command.
-
Neither source resolves (no repo_checks.* keys, no
packageManager, no recognised lockfile) → the triple is
unrunnable (see Gate Decision below).
Once resolved, the three commands are literal. Do not swap in a
different runner mid-review, do not drop the workspace-recursive flag,
do not skip a stage because "the project doesn't have that script."
Each missing script counts as unrunnable, not as pass. The chain
above selects the project's own commands (per the config-first
discipline of the stack-de-prescription release); it is NOT a license
to substitute a runner the project did not declare or detectably use.
Gate Decision
| Condition | Action |
|---|
| All three commands run AND all three exit 0 | PROCEED to stub gate; record repo-checks-GREEN:<wave-tip-commit> as evidence |
| Any command exits non-zero (red checks) | REFUSE VERIFIED — emit REVIEW APPLIED — BLOCKED (repo-checks-RED) |
| Any command did not run (unrun, missing script, runner crash) | REFUSE VERIFIED — emit REVIEW APPLIED — BLOCKED (repo-checks-UNRUN) |
| Any command is unrunnable on this workspace (e.g. resolved package manager not installed, no workspace root, no resolvable command source per the priority chain) | REFUSE VERIFIED — emit REVIEW APPLIED — BLOCKED (repo-checks-UNRUNNABLE) |
VERIFIED refused if repo checks are red/unrun on wave-tip — override
requires signed exception (W4). The signed-exception schema is
defined by W4; until W4 lands, the only override path is for an
operator to file a signed exception under the schema W4 will publish.
There is no inline operator-prompt override at this gate. Strict is
the single, unconditional mode for this gate — every project moves
through it the same way.
Recording the Evidence
When all three resolved commands pass on the wave-tip commit, write the
literal string repo-checks-GREEN:<commit-sha> to the review artifact
(the Evidence section) and to Task.verification_evidence alongside
any test-GREEN payload already written. The evidence means "the
project's resolved repo-wide lint/typecheck/test triple all exited 0 on
this commit" — it is not tied to any particular package manager. The
evidence taxonomy entry is in
skills-for-codex/references/verification-evidence.md.
When the gate refuses VERIFIED, write the closed Codex Status: BLOCKED
with workflow detail repo-checks-RED / repo-checks-UNRUN /
repo-checks-UNRUNNABLE as applicable. Do NOT promote the verdict to
APPROVED. Do NOT proceed to Step 8 verdict assignment with any
PASS-family headline.
Project-kind interaction
config.yaml may declare project_kind: standard (default) or
project_kind: prototype. The repo-wide check gate applies in both
modes. project_kind: prototype only governs the W4 PR/CI carve-outs
for direct-strategy releases; it does NOT relax local repo-check
expectations. A prototype with a red repo-wide typecheck still has
VERIFIED refused at this gate.
See ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/config-schema.md for the project_kind
specification.
Nav-actions consumer check (Mandatory, Strict-Only)
CRITICAL: For every UC affected by the current review, the agent
MUST verify two reachability conditions before any PASS-family
headline is emitted:
- The UC's Form has populated
HAS_INBOUND_ACTION edges (per W7
"Nav Actions" subsection of nacl-sa-ui/SKILL.md).
- The QA evidence for the UC references at least one natural
entrypoint path — i.e. a route reached by clicking through the
affordance, not a route entered via direct URL paste.
This check is the consumer-side read of the W7 graph rule. This skill
does not own the rule, the Cypher, or the edges; it owns the
consumer-side refusal that prevents an unreachable UC from clearing
review. Primary-owner exception for this consumer touch is declared
in the W7 plan scope_in (nacl-tl-review (consumer-side: graph-rule check — primary-owner exception, declared here)).
Scope of the check
The check applies to every affected UC of the review (typically one
UC per --be / --fe invocation, multiple for batch review). An
affected UC is exempt only when one of the following exemption flags
is set on the UseCase node in the graph:
UseCase.actor = 'SYSTEM' — machine-triggered, no user
affordance required.
UseCase.has_ui = false — no Form attached.
UseCase.entrypoint_type IN ['deep-link-only', 'embed-only'] —
intentional URL-only or embed-only UC; each such exemption
requires a signed exception under the W4 schema referencing the
operational context (invitation link, partner iframe, etc.).
An affected UC that is NOT exempt and that fails either of the two
conditions above triggers refusal.
Procedure
Condition 1 — populated nav-actions
Run the W7 reachability blocker query from
nacl-sa-ui/references/reachability.cypher § 4
(ui_reachability_blockers), scoped to the affected UCs:
// nav_actions_consumer_check
MATCH (uc:UseCase) WHERE uc.id IN $affected_uc_ids
MATCH (uc)-[:ACTOR]->(role:SystemRole)
WHERE coalesce(role.name, '') <> 'SYSTEM'
AND coalesce(uc.has_ui, true) = true
AND NOT coalesce(uc.entrypoint_type, '') IN ['deep-link-only', 'embed-only']
OPTIONAL MATCH (uc)-[:USES_FORM]->(f:Form)
OPTIONAL MATCH (c:Component)-[:HAS_INBOUND_ACTION]->(f)
WITH uc, f, collect(DISTINCT c) AS inbound_components
WHERE f IS NULL OR size(inbound_components) = 0
RETURN uc.id AS uc_id,
coalesce(f.id, '<no-form>') AS form_id,
CASE WHEN f IS NULL THEN 'no-form' ELSE 'no-inbound-action' END AS reason
Any row in the result set is a blocker. The check fails.
Condition 2 — QA evidence references a natural entrypoint
The check is satisfied when at least one QA evidence artifact for the
UC explicitly records a navigation step from a parent screen to the
UC's Form via a captured HAS_INBOUND_ACTION affordance.
The reviewer reads the QA artifacts (.tl/tasks/UC###/qa-*.md and
linked screenshots / Playwright traces). Acceptable evidence shapes:
- A Playwright trace whose first navigation step opens the parent
screen at its canonical route, followed by a
click on a locator
matching a HAS_INBOUND_ACTION.label value, followed by an
assertion at the target Form's route.
- A QA report section "Entrypoint path" listing
Parent screen → affordance label → target Form, with a screenshot of the parent
screen rendering the affordance.
- An E2E test file imported by the test suite whose journey assertion
starts from a parent route and reaches the target Form via an
affordance click — and which produced output at Step 6a above.
If no such evidence is found, the check fails for the UC.
Gate Decision
| Condition | Action |
|---|
| Both conditions hold for every non-exempt affected UC | PROCEED; record nav-actions-GREEN:<uc_id>,<uc_id>,… as evidence (one per affected UC); proceed to Step 8 with PASS-family headline allowed |
| Condition 1 fails for any non-exempt affected UC | REFUSE VERIFIED — emit REVIEW APPLIED — BLOCKED (nav-actions-missing); the Code judgment line is CHANGES REQUESTED |
| Condition 1 holds but Condition 2 fails for any non-exempt affected UC | REFUSE VERIFIED — emit REVIEW APPLIED — BLOCKED (nav-actions-no-natural-entrypoint-evidence); verdict is CHANGES REQUESTED |
UC is exempt under actor=SYSTEM / has_ui=false / entrypoint_type ∈ {deep-link-only, embed-only} AND the exemption is recorded on the UseCase node OR carried by a signed exception (W4) | EXEMPT; record nav-actions-EXEMPT:<uc_id>:<reason> and proceed |
VERIFIED refused if nav-actions are missing or the QA evidence does
not reference a natural entrypoint — override requires signed
exception (W4). The signed-exception schema is the same one used by
the W1 repo-wide check gate; until W4 lands, the only override path
is a signed exception filed under the schema W4 will publish. There
is no inline operator-prompt override at this gate.
Project-kind interaction
project_kind: prototype does NOT relax this gate. A prototype that
ships an actor-triggered UC without a populated nav-actions section
or without natural-entrypoint QA evidence still has VERIFIED refused
at this gate. project_kind governs only the W4 PR/CI carve-outs
for direct-strategy releases.
See ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/config-schema.md for project_kind and
nacl-sa-ui/references/reachability.cypher for the full query
templates.
Recording the Evidence
When the check passes for every non-exempt affected UC, write
nav-actions-GREEN:<uc-id>,<uc-id>,… (comma-separated) to the
review artifact's Evidence section and to
Task.verification_evidence alongside any test-GREEN /
repo-checks-GREEN evidence already written. For exempt UCs, write
nav-actions-EXEMPT:<uc-id>:<reason> on a separate line.
When the gate refuses, write the closed Codex Status: BLOCKED
with workflow detail nav-actions-missing or
nav-actions-no-natural-entrypoint-evidence. Do NOT promote the
verdict to APPROVED. Do NOT proceed to Step 8 verdict assignment
with any PASS-family headline.
Worked example — Project-Beta UC-100 missing-upload-button
Project-Beta UC-100 ("Upload audio") was shipped with a fully
specified FORM-Upload (fields, validation, mutation) and a working
/upload route, yet the catalog page at /catalog carried no
upload button. UC-100's review at the time emitted REVIEW COMPLETE
because the page-local Form spec was satisfied. The Nav-actions
consumer check would have caught the gap:
- Condition 1 —
nav_actions_consumer_check with
$affected_uc_ids = ['UC-100'] returns one row
{uc_id: 'UC-100', form_id: 'FORM-Upload', reason: 'no-inbound-action'}
because no Component carried a HAS_INBOUND_ACTION edge to
FORM-Upload.
- The check FAILS. Headline becomes
REVIEW APPLIED — BLOCKED (nav-actions-missing); verdict
CHANGES REQUESTED; action required: "add HAS_INBOUND_ACTION
edges per nacl-sa-ui/SKILL.md Nav Actions subsection; re-run
/nacl:sa-ui navigation to capture the catalog page upload CTA;
re-submit for review."
The methodology stops the false-PASS at the review boundary instead
of letting it ship and surface as a production-only "where is the
upload button" report.
Stub Verification Gate (Mandatory)
CRITICAL: Before the review can proceed, the agent MUST verify stubs. This gate runs before any code quality checks.
Procedure
- Read
.tl/stub-registry.json and filter entries for the current task
- Read
.tl/tasks/{id}/stub-report.md if it exists
- Scan all files listed in the result artifact for markers:
TODO, FIXME, STUB, MOCK, HACK
- Apply classification from
${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/stub-tracking-rules.md
Gate Decision
| Condition | Action |
|---|
| CRITICAL stubs found | BLOCK -- review impossible, return to developer |
| Orphaned stubs (no UC reference) | BLOCK -- all stubs must be bound to a UC |
| WARNING stubs (count <= 3) | FLAG in review, proceed with caution |
| WARNING stubs (count > 3) | VALIDATE JUSTIFICATION — each stub comment must match the ticket-reference regex (see below); any that fail force phase rollback |
| No stubs or only INFO | PROCEED normally |
Warning Stub Justification Requirement
When WARNING stub count exceeds 3, each stub justification MUST satisfy the following regex (case-insensitive):
(UC|TECH|FR|BUG)-?\d+|https?://
The agent MUST scan the comment text of every WARNING stub and test it against this pattern. If any stub comment fails the regex match:
- Set the phase status back to
in_progress.
- Halt the review and emit:
REVIEW HALTED — UNVERIFIED (stubs lack ticket references)
The following stub justifications do not contain a valid ticket ID or URL:
- <File:Line>: "<comment text>"
- ...
Required format: reference a ticket ID (e.g. UC014, TECH-042, FR-7, BUG99)
or a full URL (https://...) in each stub comment.
Action: update stub comments to include ticket references, re-run /nacl:tl-stubs, resubmit for review.
Run: /nacl:tl-dev-be UC### --continue | /nacl:tl-dev-fe UC### --continue | /nacl:tl-dev TECH### --continue
A stub comment that passes the regex but still provides no meaningful context is a MAJOR issue recorded in the review — but does not force rollback. Only the absence of any ticket/URL token triggers rollback.
Example acceptable justification:
// STUB(UC014): placeholder until payment gateway is integrated — TECH-042
Example acceptable (URL):
// TODO: replace with real API — https://linear.app/team/issue/BACK-77
Example unacceptable (free-text only — triggers rollback):
// TODO: add real implementation later
// TODO: see backlog
Frontend-Specific Stub Checks (--fe only)
- Hardcoded mock data in components (arrays with test names/emails)
// TODO: replace with real API call comments
- MSW handlers that should be removed from production code
- Placeholder images (
via.placeholder.com, /placeholder.png)
- Placeholder text (
Lorem ipsum, Test, Sample)
console.log statements in component code
If Gate BLOCKS
Set the phase status back to in_progress, display the blocking stubs, and exit:
Stub Gate: BLOCKED
Task: UC### [Title]
Reason: Critical stubs detected
| ID | File:Line | Severity | Description |
|----|-----------|----------|-------------|
| STUB-001 | src/orders/order.service.ts:45 | CRITICAL | Empty getOrders() |
Action: Resolve all CRITICAL stubs, re-run /nacl:tl-stubs, resubmit for review.
Run: /nacl:tl-dev-be UC### --continue | /nacl:tl-dev-fe UC### --continue | /nacl:tl-dev TECH### --continue
Workflow
Step 1: Read Task Files
Read ALL relevant files for the identified review mode.
Backend (--be):
.tl/tasks/UC###/
task-be.md # What was supposed to be implemented (backend)
test-spec.md # Expected test cases
impl-brief.md # Implementation guidelines
acceptance.md # Acceptance criteria to verify
result-be.md # Development results to review
Frontend (--fe):
.tl/tasks/UC###/
task-fe.md # What was supposed to be implemented (frontend)
test-spec-fe.md # Expected RTL test cases
impl-brief-fe.md # UI implementation guidelines
acceptance.md # Acceptance criteria to verify
result-fe.md # Development results to review
TECH (no flag):
.tl/tasks/TECH###/
task.md # What was supposed to be implemented
result.md # Development results to review
Step 2: Update Status to in_review
Set the appropriate phase in status.json:
- BE:
phases.review_be = "in_review"
- FE:
phases.review_fe = "in_review"
- TECH:
status = "in_review"
Step 3: Verify Acceptance Criteria (requirements traceability)
Enumerate every acceptance criterion / REQ in acceptance.md as its own row — do not collapse them into the category groups (Functional, Business Rules, Error Handling, Performance, Security). Category-grouped review lets a single missing requirement hide inside a mostly-passing group; a per-criterion pass is the cheapest defence against the "missing-requirement" defect class (e.g. an AI-call audit log the spec demands but no code writes).
For each criterion, establish three facts before scoring:
- Implemented? the code under review actually does what the criterion requires — confirmed by reading the runtime that produces the behaviour (Step 4a cross-file trace), not by the presence of a plausibly-named function. A "feature" that calls the wrong runtime (dead config) is NOT implemented.
- Reachable? the behaviour is reachable end-to-end from a real entrypoint — for UI-affecting criteria reuse the Nav-actions natural-entrypoint evidence (see the Nav-actions consumer check) as the reachability witness.
- Tested? a test or QA path exercises it.
Score each criterion PASS / PARTIAL / FAIL. A criterion implemented but unreachable — or implemented but untested where the spec demands a test — is at most PARTIAL. A criterion not implemented, or implemented against the wrong runtime, is FAIL. PARTIAL means partly met with a gap that does not block (e.g., happy path covered but an edge case missing); the verdict aggregator decides promotion. If any of the three facts (implemented / reachable / tested) cannot be positively confirmed from the code or tests, score the criterion at most PARTIAL and record the unconfirmed fact — never PASS on absence of evidence.
Step 4: Code Quality Review
Apply the appropriate checklist (see detailed checklists below).
4a. Cross-file trace (Mandatory)
The change under review is the diff, but you MUST judge it in context — read the files the diff depends on and the files that depend on it, using the actual repo (not just the diff hunks):
- Imports / callees: for each symbol the diff calls, open the file that defines it and confirm the diff's assumptions about its behaviour hold — signature, return shape, side effects, and any hardcoded value that silently overrides a parameter.
- Callers / consumers: for each symbol the diff changes (a renamed field, a new return shape, a changed enum), grep the source roots (the same roots Step 2.5 uses) for its call-sites and confirm every consumer still works.
- Runtime that produces the data: trace back to the service/client/migration that actually produces a value the diff consumes — a field can be declared in the contract or config yet never be set by the runtime (dead config), so a "feature" reads as implemented while the runtime does something else.
Cross-file and missing-integration defects are exactly what review must catch; do NOT restrict yourself to the diff hunks. A BLOCKER/CRITICAL finding may be refuted only with positive cross-file evidence — you read the guard/handler/consumer/runtime that makes it a non-issue; never downgrade or dismiss a high-severity finding on the strength of the diff alone, or on uncertainty (see the adversarial-verify-needs-context rule). Emit any cross-file defect as a normal finding using the existing severity vocabulary (no new status).
Step 5: Verify TDD Compliance
Check result file for evidence of TDD phases:
| Phase | Evidence Required |
|---|
| RED | Tests written before implementation, tests failed initially |
| GREEN | Minimal implementation, all tests passed |
| REFACTOR | Code improved, tests still passed |
Verify commits follow the pattern:
test(UC###): ... for RED phase
feat(UC###): ... for GREEN phase
refactor(UC###): ... for REFACTOR phase
Step 6: Run Tests and Check Author Independence
6a. Run the test suite (declared scripts.test only)
Execute the workspace's declared scripts.test. Do NOT fall back to npm test, npx jest, npx vitest, or any other invented command (Cross-cutting principle P2). The runner is exactly what the workspace declares.
If scripts.test is missing → halt as REVIEW HALTED — NO_INFRA (scripts.test undeclared). Do NOT promote the verdict to APPROVED; do NOT proceed to Step 8 verdict assignment with a PASS-family headline. Operator override is permitted but the headline becomes REVIEW APPLIED — UNVERIFIED (no test infra) and the verdict cannot be APPROVED.
If the runner crashes (non-zero exit before any test runs, or stderr non-empty with empty stdout) → halt as REVIEW HALTED — RUNNER_BROKEN. Same APPROVED-prohibition applies.
For the runs that produce output, capture:
- Pass/fail counts
- Coverage percentage
- Any flaky test indicators
6a-baseline. Baseline capture for "new failures" claims
This skill runs after a change has landed, so a single working-tree run is postfix-only. Any "tests revealed new failures" claim requires an explicit baseline (Cross-cutting principle P3).
Resolve a baseline ref in priority order:
--base <ref> flag passed to this skill.
- Saved baseline artifact
.tl/tasks/UC###/baseline-failures-{be,fe}.json written by the upstream dev skill at its CAPTURE BASELINE step.
- Default:
git merge-base HEAD main (or the configured git.main_branch).
Run the baseline using git worktree add (mirroring nacl-tl-sync Step 7.2):
git worktree add <tempdir> <baseline_ref>
cd <tempdir> && <scripts.test>
git worktree remove -f <tempdir>
Capture baseline_failures and compute new_failures = postfix_failures − baseline_failures. The worktree is removed on every exit path.
If no baseline ref resolves → record UNVERIFIED (no baseline) for the workspace. Do NOT classify any failure as "new" or "pre-existing" — set arithmetic is undefined when one operand is missing.
Verify:
- All tests pass (via postfix run)
- Coverage meets thresholds (80%+ recommended)
- No flaky tests
- "New failures" claim is set-arithmetic-derived, not authored from the postfix run alone
6b. Test Author Independence Check
Goal: Detect when tests were written by the same person who wrote the production code for this UC, which reduces the independence guarantee of the test suite.
Procedure:
- Identify the test files for the UC under review (from
result-be.md or result-fe.md).
- Run
git log --format="%ae" -- <test_file> for each test file to collect author emails.
- Run
git log --format="%ae" -- <production_file> for each production source file to collect author emails.
- Compute overlap: what fraction of test-file commits share an author with production-file commits for the same UC?
Single-identity pre-check (before computing overlap): run
git log --format="%ae" | sort -u for the whole repository. If every
commit shares one author identity — the normal case for agent-driven
development, where all agents commit as the same git user — the overlap
metric is trivially 100% and carries no signal. Do NOT raise the MAJOR
flag from it. Instead record in the review artifact:
Test author independence: uninformative (single-identity repo)
and verify the structural seam in its place: the dev result
(result-be.md / result-fe.md / result.md) must show the regression
test was written through the test-author sub-agent seam (the
nacl-tl-regression-test skill, invoked as a separate sub-agent — the
fix/feature author is never the test author). Missing seam evidence on a
single-identity repo IS a MAJOR flag (same report block and Recommend
line as below).
Classification (multi-identity repos):
| Overlap | Action |
|---|
| <= 50% | No flag — sufficient independence |
| > 50% | MAJOR flag — test and production code share the same primary author for this UC |
When MAJOR flag is raised:
Add to the review report:
MAJOR: Test Author Independence
Test files and production files for this UC are authored predominantly by the same
contributor (overlap: N%). This reduces the independence guarantee — the same
author may have unconsciously tuned both the implementation and the tests.
This flag does NOT block approval, but it must be visible in the review report.
In addition, the "Next Steps" section of the review artifact MUST include this exact line (substituting the UC identifier):
Recommend: `/nacl:tl-regression-test --retroactive UC###`
Downstream contract: nacl-tl-ship and nacl-tl-deliver MAY read the MAJOR test-author flag from the review artifact and gate progression on it (e.g., require the retroactive regression test to be filed before shipping). Whether they enforce this gate is their decision; this skill's obligation is to surface the flag and the recommendation so downstream consumers can act on it.
This check is non-blocking at the review layer. A MAJOR flag does not prevent REVIEW COMPLETE or APPROVED, but it must appear in the review artifact and is visible to downstream consumers.
Relation to the Step 8b headline and P4: the test-author flag does NOT
change the headline. The headline reflects the completeness of the
verification (per 8b's preamble), and an author-overlap flag does not make
the verification incomplete — the tests ran, passed, and carry RED→GREEN
evidence. With everything else green the headline remains REVIEW COMPLETE
and APPROVED remains reachable under P4. This is NOT the removed
"proceed but flag in report" loophole returning: P4 is untouched — a
non-REVIEW COMPLETE headline still forbids APPROVED; the author-overlap
signal simply is not a headline condition. Its enforcement surface is the
review artifact (the MAJOR block + the mandatory Recommend line) and the
downstream ship/deliver gates described above. Rationale for non-blocking:
author overlap is a git-history fact — re-running the dev agent cannot
clear it, so a blocking reading has no terminating path through the
orchestrator retry loops.
Step 7: Document Issues
Categorize by severity: Blocker (must fix), Critical (should fix), Major (should fix), Minor (nice to have). For each issue document file, line, description, recommended fix, rationale. A finding's severity may be lowered, or the finding dropped, only on positive evidence it is a non-issue (you read the guard/handler/consumer/requirement that resolves it); on uncertainty keep it at its assessed severity and flag the open question (QUESTION) rather than silently dropping or downgrading it (adversarial-verify-needs-context).
7a. Self-adversarial pass (Mandatory for BLOCKER/CRITICAL)
Before assigning the verdict, take each BLOCKER and CRITICAL finding and try to refute your own claim: re-read the actual code path it cites — and its callers/consumers/runtime (Step 4a) — one more time, asking "what would make this a non-issue?". This is the single-agent analogue of an independent verifier: a cheap second look that kills false positives without a second reviewer. Drop a finding here only if the second read produces positive evidence it is wrong (a guard/handler you missed, a consumer that never hits the path); on uncertainty, keep it (Step 7's evidence rule — never drop on doubt). Log what you re-read for each finding you drop.
Step 8: Make Decision and Assign Headline
8a. Determine review verdict
| Result | Condition | Status Update |
|---|
approved | No blockers, all criteria met | Phase -> approved |
rejected | Blockers found or stub gate failed | Phase -> in_progress |
8b. Assign headline status
The headline is independent of the APPROVED / CHANGES REQUESTED verdict. It reflects the completeness of the verification, not the quality judgment.
| Condition | Headline | APPROVED allowed? |
|---|
Repo-wide gate GREEN AND nav-actions check GREEN/EXEMPT AND tests ran AND passed AND no warnings above threshold AND baseline resolved AND new_failures.size == 0 | REVIEW COMPLETE | yes |
| Repo-wide gate RED / UNRUN / UNRUNNABLE on wave-tip | REVIEW APPLIED — BLOCKED (repo-checks-*) | no — VERIFIED refused; override requires signed exception (W4) |
| Nav-actions check fails (Condition 1: missing HAS_INBOUND_ACTION on non-exempt affected UC) | REVIEW APPLIED — BLOCKED (nav-actions-missing) | no — VERIFIED refused; override requires signed exception (W4) |
| Nav-actions check fails (Condition 2: QA evidence has no natural-entrypoint path) | REVIEW APPLIED — BLOCKED (nav-actions-no-natural-entrypoint-evidence) | no — VERIFIED refused; override requires signed exception (W4) |
| Tests ran AND passed AND no test imports the changed file(s) | REVIEW APPLIED — UNVERIFIED | no |
| Tests ran AND postfix has failures BUT baseline could not be resolved | REVIEW APPLIED — UNVERIFIED (no baseline) | no |
scripts.test missing | REVIEW HALTED — NO_INFRA | no — APPROVED forbidden |
| Runner crashed | REVIEW HALTED — RUNNER_BROKEN | no — APPROVED forbidden |
| Operator override under NO_INFRA / RUNNER_BROKEN | REVIEW APPLIED — UNVERIFIED (no test infra) | no |
new_failures.size > 0 (baseline resolved) | REVIEW INCOMPLETE — REGRESSION | no |
postfix_failures ⊆ baseline_failures AND postfix_failures.size > 0 (baseline resolved) | REVIEW APPLIED — BLOCKED (pre-existing failures) | operator-gated |
| Review blocked (CRITICAL stubs, or prerequisite unmet) | REVIEW APPLIED — BLOCKED | no |
APPROVED promotion rule (P4): the verdict line Code judgment: APPROVED may only be written when the headline is REVIEW COMPLETE. Any other headline forbids APPROVED; the verdict line MUST be Code judgment: CHANGES REQUESTED (or Code judgment: BLOCKED). The previous "proceed to review but flag in report" loophole is removed.
Both the headline and the APPROVED / CHANGES REQUESTED verdict appear in the review artifact as a single combined status line in this format:
Workflow status: `REVIEW COMPLETE`. Code judgment: `APPROVED`. Action required: none.
or when issues exist:
Workflow status: `REVIEW COMPLETE`. Code judgment: `CHANGES REQUESTED`. Action required: address 3 BE-3 stub-justifications.
or when the runner could not fully verify (P4: a non-REVIEW COMPLETE
headline forbids APPROVED):
Workflow status: `REVIEW APPLIED — UNVERIFIED (no test imports the changed files)`. Code judgment: `CHANGES REQUESTED`. Action required: `/nacl:tl-regression-test --retroactive UC###`.
or when everything is green but the test-author flag was raised (the flag
does not change the headline — see Step 6b):
Workflow status: `REVIEW COMPLETE`. Code judgment: `APPROVED`. Action required: recommended — retroactive regression test (see Next Steps).
The "Action required" field summarises the most urgent next step. If there are none, write none. This line MUST appear as the first content line of the review artifact's summary section.
Step 9: Create Review Artifact
Write to review-be.md (BE), review-fe.md (FE), or review.md (TECH) using ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/templates/review-template.md. Include: summary, stub gate result, files reviewed, acceptance verification, checklist findings, issues, test results, TDD compliance, test author independence result, positive observations, headline + verdict, next steps.
Step 10: Update Tracking
Update status.json based on decision.
If Approved (BE):
{
"phases": {
"review_be": "approved"
},
"review_be_completed": "YYYY-MM-DDTHH:MM:SSZ",
"review_be_result": "approved"
}
If Approved (FE):
{
"phases": {
"review_fe": "approved"
},
"review_fe_completed": "YYYY-MM-DDTHH:MM:SSZ",
"review_fe_result": "approved"
}
If Approved (TECH):
{
"status": "approved",
"reviewed": "YYYY-MM-DDTHH:MM:SSZ",
"review_result": "approved"
}
If Rejected (any mode): set phase/status to in_progress, record review_result: "rejected", list blockers array.
Append to changelog.md:
## [YYYY-MM-DD HH:MM] REVIEW: UC### - Title (BE/FE/TECH)
- Status: Workflow `REVIEW COMPLETE`. Judgment `APPROVED`. Action required: none.
- Stub Gate: PASSED / BLOCKED / WARNING (N) / HALTED — UNVERIFIED
- Result: approved / rejected
- Issues: N blocker, N critical, N major, N minor
- Tests: N passed, coverage X%
- Test author independence: OK / MAJOR (N% overlap)
- Checklist PARTIAL rows: N (details in artifact)
Backend Review Checklist (8 Categories)
Used for --be reviews and TECH reviews. Reference: ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/review-checklist.md.
Each checklist row uses tri-state scoring:
- PASS — fully satisfied
- PARTIAL — partly satisfied; surfaces in the report but does not automatically block approval; the verdict aggregator decides
- FAIL — not satisfied; treated as a blocker or critical issue depending on category
1. Code Correctness
2. Code Quality
3. Error Handling
4. Testing
5. Security
6. Performance
7. Documentation
8. Git and Commits
Frontend Review Checklist (10 Categories)
Used for --fe reviews. Reference: ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/fe-review-checklist.md.
Each checklist row uses tri-state scoring:
- PASS — fully satisfied
- PARTIAL — partly satisfied; surfaces in the report but does not automatically block approval; the verdict aggregator decides
- FAIL — not satisfied; treated as a blocker or critical issue depending on category
1. Component Architecture
2. TypeScript Quality
3. State Management
4. API Integration
5. Forms and Validation
6. Accessibility
7. Responsive Design
8. Performance
9. Testing (RTL)
10. Stubs/Mocks Cleanup
Feedback Guidelines
Use these prefixes for clarity in review comments:
| Type | Prefix | Example |
|---|
| Must fix | BLOCKER | "This will cause a null pointer exception when user is null" |
| Should fix | CRITICAL | "Consider adding cache invalidation after this mutation" |
| Should fix | MAJOR | "Missing test for error case in order creation flow" |
| Nice to have | MINOR | "Consider renaming d to orderDate for clarity" |
| Clarification | QUESTION | "Could you explain the rationale for this approach?" |
| Encouragement | PRAISE | "Great use of discriminated unions here!" |
Constructive Feedback Examples
BAD: "This is wrong"
GOOD: "This might throw when `user` is null. Consider adding a null check"
BAD: "Bad naming"
GOOD: "Consider renaming `d` to `orderDate` for clarity"
BAD: "Fix this"
GOOD: "The test checks implementation details. Consider testing behavior instead"
Output Summary
Backend Review
Backend Code Review Complete
Task: UC### [Title] (Backend)
Workflow status: `REVIEW COMPLETE`. Code judgment: `APPROVED`. Action required: none.
Stub Check: No critical stubs / N warnings (non-blocking)
Test Author Independence: OK / MAJOR (N% overlap — retroactive /nacl:tl-regression-test recommended)
BE Checklist:
Code Correctness: PASS/PARTIAL/FAIL Error Handling: PASS/PARTIAL/FAIL
Code Quality: PASS/PARTIAL/FAIL Testing: PASS/PARTIAL/FAIL
Security: PASS/PARTIAL/FAIL Performance: PASS/PARTIAL/FAIL
Documentation: PASS/PARTIAL/FAIL Git & Commits: PASS/PARTIAL/FAIL
Issues: N blocker, N critical, N major, N minor
Tests: N passed, coverage N%
Next: /nacl:tl-dev-fe UC### (start frontend) or /nacl:tl-sync UC### (verify sync)
Frontend Review
Frontend Code Review Complete
Task: UC### [Title] (Frontend)
Workflow status: `REVIEW COMPLETE`. Code judgment: `APPROVED`. Action required: none.
Stub Check: No critical stubs / N warnings (non-blocking)
Test Author Independence: OK / MAJOR (N% overlap — retroactive /nacl:tl-regression-test recommended)
FE Checklist:
Component Architecture: PASS/PARTIAL/FAIL API Integration: PASS/PARTIAL/FAIL
TypeScript Quality: PASS/PARTIAL/FAIL Forms & Validation: PASS/PARTIAL/FAIL
State Management: PASS/PARTIAL/FAIL Accessibility: PASS/PARTIAL/FAIL
Responsive Design: PASS/PARTIAL/FAIL Performance: PASS/PARTIAL/FAIL
Testing (RTL): PASS/PARTIAL/FAIL Stubs/Mocks Cleanup: PASS/PARTIAL/FAIL
Issues: N blocker, N critical, N major, N minor
Tests: N passed, coverage N%
Next: /nacl:tl-sync UC### (verify BE<>FE sync) or /nacl:tl-qa UC### (E2E testing)
TECH Review
TECH Code Review Complete
Task: TECH### [Title]
Workflow status: `REVIEW COMPLETE`. Code judgment: `APPROVED`. Action required: none.
Stub Check: No critical stubs
BE Checklist (applied to TECH): All PASS / N PARTIAL / N FAIL
Test Author Independence: OK / MAJOR (N% overlap)
Next: /nacl:tl-status or /nacl:tl-next
If Rejected (Any Mode)
Code Review: CHANGES REQUESTED
Task: UC### [Title] (Backend/Frontend) or TECH### [Title]
Headline: REVIEW INCOMPLETE — REGRESSION / REVIEW APPLIED — BLOCKED / ...
Blockers Found:
B01: [description]
B02: [description]
Stub Check: N critical stubs (if applicable)
Run: /nacl:tl-dev-be UC### --continue (BE rejections)
Run: /nacl:tl-dev-fe UC### --continue (FE rejections)
Run: /nacl:tl-dev TECH### --continue (TECH rejections)
Review Decision Flow
If APPROVED
- BE approved ->
phases.review_be = "approved"
- FE approved ->
phases.review_fe = "approved"
- TECH approved ->
status = "approved"
If CHANGES REQUESTED
Status reverts to in_progress; developer must use --continue:
- BE rejected ->
/nacl:tl-dev-be UC### --continue (reads review-be.md)
- FE rejected ->
/nacl:tl-dev-fe UC### --continue (reads review-fe.md)
- TECH rejected ->
/nacl:tl-dev TECH### --continue (reads review.md)
Note on rejection path: distinguish the cause clearly.
- If tests fail because the implementation is wrong →
REVIEW INCOMPLETE — REGRESSION; return to implementation.
- If tests fail because tests were written to match a buggy implementation →
REVIEW APPLIED — UNVERIFIED + MAJOR flag; recommend /nacl:tl-regression-test before re-review.
Error Handling
Task Not Found
If task files do not exist:
Error: Task {id} not found or not ready for review
Expected structure (--be):
.tl/tasks/UC###/
task-be.md, test-spec.md, impl-brief.md, acceptance.md
result-be.md <-- Required for review
Expected structure (--fe):
.tl/tasks/UC###/
task-fe.md, test-spec-fe.md, impl-brief-fe.md, acceptance.md
result-fe.md <-- Required for review
Expected structure (TECH):
.tl/tasks/TECH###/
task.md, result.md <-- Required for review
Run the appropriate development skill first:
/nacl:tl-dev-be UC### (backend)
/nacl:tl-dev-fe UC### (frontend)
/nacl:tl-dev TECH### (TECH tasks)
Not Ready for Review
Error: Task UC### is not ready for review
Current phase status: {{status}}
Expected: ready_for_review
Complete development first:
/nacl:tl-dev-be UC### | /nacl:tl-dev-fe UC### | /nacl:tl-dev TECH###
Missing Flag for UC Tasks
Error: Review mode flag required for UC tasks
Usage:
/nacl:tl-review UC### --be (review backend code)
/nacl:tl-review UC### --fe (review frontend code)
/nacl:tl-review TECH### (TECH task, no flag needed)
Tests Fail
Warning: Tests are failing (Passed: N, Failed: N)
Headline will be REVIEW INCOMPLETE — REGRESSION. Review continues to identify all issues.
Distinguish: is the implementation wrong, or are the tests tuned to a buggy implementation?
Missing Result File
Error: Development results not found
Missing: .tl/tasks/UC###/result-be.md (or result-fe.md / result.md)
Run: /nacl:tl-dev-be UC### | /nacl:tl-dev-fe UC### | /nacl:tl-dev TECH###
Essential Quick Checks (All Modes)
Before deep review, verify these first:
Reference Documents
| Task | Reference |
|---|
| BE review checklist | ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/review-checklist.md |
| FE review checklist | ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/fe-review-checklist.md |
| Stub tracking | ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/stub-tracking-rules.md |
| Code style (BE) | ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/code-style.md |
| FE code style | ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/fe-code-style.md |
| TDD workflow | ${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/references/tdd-workflow.md |
Templates
${CLAUDE_PLUGIN_ROOT}/nacl-tl-core/templates/review-template.md -- Review result template
Procedural Checklist
Before Starting
Repo-wide Check Gate
Nav-actions Consumer Check (W7)
Stub Gate
During Review
After Review
Next Steps
BE Approved: /nacl:tl-dev-fe UC### (start FE) or /nacl:tl-sync UC### (verify sync)
FE Approved: /nacl:tl-sync UC### (sync check) or /nacl:tl-qa UC### (E2E testing)
TECH Approved: /nacl:tl-docs TECH### (documentation) or /nacl:tl-next (next task)
Any Rejected: /nacl:tl-dev-be UC### --continue | /nacl:tl-dev-fe UC### --continue | /nacl:tl-dev TECH### --continue