ln-402-task-reviewer

What this skill does

> **Paths:** File paths (`references/`, `../ln-*`) are relative to this skill directory.

**Type:** L3 Worker
**Category:** 4XX Execution

# Task Reviewer

**MANDATORY after every task execution.** Reviews a single task in To Review and decides Done vs To Rework with immediate fixes or clear rework notes.

> **This skill is NOT optional.** Every executed task MUST be reviewed immediately. No exceptions, no batching, no skipping.

## Purpose & Scope
- Resolve task ID (per Input Resolution Chain); load full task and parent Story independently via the configured tracker provider (`getTask`, `getStory`).
- Check architecture, correctness, configuration hygiene, docs, and tests.
- For test tasks, verify risk-based limits and priority (≤15) per planner template.
- Update only this task: accept (Done) or send back (To Rework) with explicit reasons and fix suggestions tied to best practices.

## Inputs

| Input | Required | Source | Description |
|-------|----------|--------|-------------|
| `taskId` | Yes | args, parent Story, kanban, user | Task to review |

**Resolution:** Task Resolution Chain.
**Status filter:** To Review

## Phase 0: Tools Config

**MANDATORY READ:** Load `references/environment_state_contract.md`, `references/storage_mode_detection.md`, and `references/input_resolution_pattern.md`

Extract: `task_provider` = Task Management -> Provider (`linear` | `github` | `file`). Operations stay provider-agnostic; load the selected provider transport reference only when performing tracker I/O.

## Task Storage Mode

Tracker operations used by this skill: `getTask`, `getStory`, `updateStatus` (Done | To Rework), `addComment`, `createTask` (for [BUG] side-effect tasks).

## Mode Detection

Detect operating mode at startup:

**Plan Mode Active:**
- Steps 1-3: Resolve task and load context (read-only, OK in plan mode)
- Generate REVIEW PLAN (files, checks) → write to plan file
- Call ExitPlanMode → STOP. Do NOT execute review.
- Steps 4-9: After approval → execute full review

**Normal Mode:**
- Steps 1-9: Standard workflow without stopping

## Plan Mode Support

Conditional read: load `references/plan_mode_pattern.md` Workflow A only when Plan Mode is active.
Tool policy: follow host AGENTS.md MCP preferences; load `references/mcp_tool_preferences.md` and `references/mcp_integration_patterns.md` only when host policy is absent or MCP behavior is unclear.

**CRITICAL: In Plan Mode, plan file = REVIEW PLAN (what will be checked). NEVER write review findings or verdicts to plan file.**

**Review Plan format:**

```
REVIEW PLAN for Task {ID}: {Title}

| Field | Value |
|-------|-------|
| Task | {ID}: {Title} |
| Status | {To Review} |
| Type | {impl/test/refactor} |
| Story | {Parent ID}: {Parent Title} |

Files to review:
- {file1} (deliverable)
- {file2} (affected component)

| # | Check | Will Verify |
|---|-------|-------------|
| 1 | Approach | Technical Approach alignment |
| 2 | Clean Code | No dead code, no backward compat shims |
| 3 | Config | No hardcoded creds/URLs |
| 4 | Errors | try/catch on external calls |
| 5 | Logging | ERROR/INFO/DEBUG levels |
| 6 | Comments | WHY not WHAT, docstrings |
| 7 | Naming | Project conventions |
| 8 | Docs | API/env/README updates |
| 9 | Tests | Updated/risk-based limits |
| 10 | AC | 4 criteria validation |
| 11 | Side-effects | Pre-existing bugs in touched files |
| 12 | Destructive ops | Safety guards from destructive_operation_safety.md (loaded in step 4) |
| 13 | Algorithm correctness | Loop invariants, collection keys, unbounded ops, shared state leaks |
| 14 | Event channels | Channel name consistency in diff |
| 15 | CI Checks | lint/typecheck pass per ci_tool_detection.md |

Expected output: Verdict (`Done | To Rework`) + Issues + Fix actions
```

## Progress Tracking with TodoWrite

When operating in any mode, skill MUST create detailed todo checklist tracking ALL steps.

**Rules:**
1. Create todos IMMEDIATELY before Step 1
2. Each workflow step = separate todo item; multi-check steps get sub-items
3. Mark `in_progress` before starting step, `completed` after finishing

**Todo Template (~11 items):**

```
Step 1: Resolve taskId
  - Resolve via args / Story context / kanban / AskUserQuestion (To Review filter)

Step 2: Load Task
  - Load task by ID, detect type

Step 3: Read Context
  - Load full task + parent Story + affected components

Step 3b: Goal Articulation Gate
  - State what specific quality question this review must answer (<=25 tokens each)

Step 4: Review Checks
  - Verify approach alignment with Story Technical Approach
  - Check clean code: no dead code, no backward compat shims
  - Cross-file DRY: Grep src/ for new function/class names (count mode). 3+ similar → CONCERN
  - Check config hygiene, error handling, logging
  - Check comments, naming, docs updates
  - Verify tests updated/run (risk-based limits for test tasks)

Step 5: AC Validation
  - Validate implementation against 4 AC criteria

Step 6: Side-Effect Bug Detection
  - Scan for bugs outside task scope, create [BUG] tasks

Step 7: Decision
  - Apply minor fixes or set To Rework with guidance

Step 8: Mechanical Verification
  - Run lint/typecheck per ci_tool_detection.md (only if verdict=Done)

Step 9: Update & Commit
  - Set task status, update kanban, post review comment
  - If Done: leave branch changes uncommitted for downstream branch ownership rules
```

## Workflow (concise)
Use `hex-graph` first when semantic diff, clone groups, references, or review blast radius matter. Use `hex-line` first for local code/config/script/test reads when available. If MCP is unavailable, unsupported, or not indexed, continue with built-in `Read/Grep/Glob/Bash` and record the fallback in the review instead of blocking.

1) **Resolve taskId:** Run Task Resolution Chain per guide (status filter: [To Review]).
2) **Load task:** Load full task and parent Story independently. Detect type (label "tests" -> test task, else implementation/refactor).
3) **Read context:** Full task + parent Story; load affected components/docs; review diffs if available.
   **Hex MCP acceleration:** Prefer `analyze_changes(path=project_root, base_ref="HEAD~1")` for semantic risk snapshot when graph is indexed; use `changes(path="src/", compare_against="HEAD~1")` for AST-level diff review of structural changes.
3b) **Goal gate:** Before reviewing, state: (1) REAL GOAL: what quality question must this review answer? (2) DONE: what evidence proves quality is sufficient? (3) NOT THE GOAL: what would a surface-level rubber-stamp look like? (4) INVARIANTS: what non-obvious constraint exists? Load `references/goal_articulation_gate.md` only when this gate is ambiguous or disputed.
4) **Review checks:**
   > **Spec-first gate:** Quick AC pre-check: scan task AC against implementation. If any AC is clearly unmet (BLOCKER-level) → immediate To Rework, skip remaining quality checks. Full AC validation still runs in Step 5.
   **MANDATORY READ:** Load `references/clean_code_checklist.md`, `references/destructive_operation_safety.md`
   - **Goal validation (Recovery Paradox):** If executor articulated a REAL GOAL (visible in task comments or implementation), validate it matches the Story's target deliverable. If executor framed the goal around a secondary subject (e.g., "implement the endpoint" instead of "enable user data export") → CONCERN: `GOAL-MISFRAME: executor goal targets secondary subject, may miss hidden constraints.`
   - **Blueprint completion (advisory):** If executor runtime data available (`.hex-skills/runtime-artifacts/runs/` for this task), load PHASE_3 blueprint and PHASE_6 `blueprint_status` from executor checkpoints. Flag as CONCERN if: `completion_pct < 100` without justifications for skipped items, or added files exceed 50% of planned without justification. If runtime data unavailable, check `metadata.blueprint_status` from executor summary. Not a BLOCKER.
   - Approach: diff aligned with Technical Approach in Story. If different → rationale do