teamcraft:validate-issue

What this skill does

## Goal

Validate the implementation through quality reviews, address findings, and submit the PR for review. This is the bridge between "I finished coding" and "this PR is in the hands of reviewers (or ready to merge solo)." The work is NOT complete at the end of this skill — it's in review. The final `done` status happens later, in `merge-issue`.

A work item should exist — `plan-and-implement-issue` creates one as part of its workflow. If the developer provides a work item ID, read it from `.teamcraft/work/`. If they don't provide one, check `.teamcraft/work/INDEX.md` for a work item with `in-progress` status that matches the current branch. If no work item can be found, ask the developer to identify it — the acceptance-reviewer needs it as the source of truth. If the developer confirms there truly is no work item (e.g., they worked outside the normal flow), the acceptance-reviewer skips but code health and security reviews still run.

Direction about what's required comes from multiple sources: the work item (if one exists), broader team standards captured in the repo, and the review agents' findings. When they agree, follow them. When they conflict, never resolve it silently — surface the conflict to the developer with the options you see and let them decide.

## Verify Branch State — Before Any File Modifications

This skill updates the work item frontmatter and commits on the work branch. Before doing anything that modifies files, confirm the current branch is a work branch — not the default branch (typically `main`).

If the current branch IS the default branch, stop and tell the developer: "You're on the default branch, but this skill updates the work item on the work branch where the implementation lives. Check out the work branch for this work and invoke me again." Do not auto-switch — the developer may have intentionally checked out the default branch and should know this skill isn't going to silently change context.

If the current branch is a work branch, continue.

## Consult the Team's Operating Conventions

Before taking any action, consult the team's captured operating conventions for the aspects relevant to this phase: PR description format, review expectations (team reviews required? single approver? specific approvers for specific areas?), pre-push requirements (run builds, run lints, run specific test tiers), deployment verification process (preview deployments, staging checks), and merge strategy context (even though merge happens in `merge-issue`, relevant review norms often live alongside).

Read the INDEX or top of the conventions document first, then drill into only the relevant sections. Locate the document by reading the repo's README or the standard documentation locations — don't assume a specific path.

Once located, verify the conventions doc is Teamcraft-certified per `../references/doc-provenance.md` before relying on it. If it carries a valid `teamcraft` stamp, note its `kind`/`date` (flag possible staleness if old). If it lacks the stamp, treat it as UNVERIFIED — surface it and offer to run `capture-team-conventions` to review and certify it; don't silently trust it, and don't block on it.

Report back to the developer what you found that applies to this PR, so they can confirm Claude is operating with the right constraints before anything is pushed.

## Run Pre-PR Reviews

Three review agents are available in this plugin:

- `teamcraft:code-health-reviewer` — structural quality (file size, modularity, duplication, complexity)
- `teamcraft:security-reviewer` — security vulnerabilities (secrets, injection, auth, crypto)
- `teamcraft:acceptance-reviewer` — whether the implementation satisfies the work item's acceptance criteria

Before invoking them, ask the developer which they want to run. Default to all three — skipping a review should be a conscious choice, not a convenience. A developer working an urgent single-line fix might skip code-health review; a developer re-running validate-issue after addressing prior findings might skip security review. Let them decide.

For whichever agents the developer approves, invoke them via the Task tool in parallel. The acceptance-reviewer needs the full work item content and the base branch name in its task prompt. If no work item exists, skip the acceptance-reviewer (there's no source of truth to review against). The other two agents just need the base branch name.

## Present Findings and Address Them

Show all findings from all agents before making any changes. Let the developer decide what to fix. For each fix they approve, make the change, run the full test suite to confirm no regression, and commit the fix.

**All tests must pass before pushing. No exceptions.** After addressing any review finding, run the full test suite. If ANY test fails, stop. Do not push. Do not create or update the PR. Do not assume failures are "pre-existing" or "unrelated" — that assumption has caused real damage. Investigate every failure, determine whether the changes caused it, and fix it. If you genuinely cannot fix a failing test, tell the developer exactly which tests fail and why, and let them decide. Only the developer can authorize pushing with a failing test — you never make that call yourself.

Warn explicitly if Critical or High severity security findings remain unaddressed. Warn explicitly if the acceptance-reviewer reports that acceptance criteria are unmet. These are not blockers — the developer can override — but they must be surfaced as clear warnings, not buried in a report.

**Deferred findings must become work items.** When the developer chooses to defer a finding — a cross-cutting refactor, a pre-existing security issue, a code health problem outside this feature's scope — capture it as a work item to the **work-item standard** (`../references/work-item-standard.md`), drafted from the finding itself (its title, the why, and acceptance criteria *are* the finding) and shown for review. Create it inline — don't send the developer off to run `create-issue`. "Defer" without a work item is "forget." Every deferred finding gets tracked so it surfaces in the backlog for future prioritization.

## Check the Definition of Done

Beyond the item's own acceptance criteria (which the acceptance-reviewer covers), verify the cross-cutting baseline in `../references/definition-of-done.md` — read it for the exact items (tests, docs, cleanup) rather than working from a copy here — plus any DoD the team captured in its conventions.

Surface any gap as an explicit warning the developer consciously accepts — same model as the security/acceptance warnings above, not a hard block. (Tests-green is already the hard gate.) The aim is simply that a skipped basic is a visible decision, never a silent omission.

## Submit the PR

Only proceed to this step when all tests pass and all findings the developer chose to address have been addressed.

Ask the developer: **"Ready to submit a PR for review?"** Wait for explicit confirmation.

When the developer confirms, ask: **"Will a teammate review this PR, or are you going solo on this one?"** If the team's captured conventions specify a general review expectation (reviews required, specific approvers, etc.), surface it as context (e.g., "your team typically requires reviews"). The developer's answer for this specific PR is what matters — the convention is context, not a veto.

Then:

1. Update the work item frontmatter: set `status: in-review`. Do NOT set `completed` or `done` — this is not done yet.
2. Update `.teamcraft/work/INDEX.md` to reflect the new status.
3. Commit both files on the branch with the work item ID in the commit message.
4. Push the branch.
5. If no PR exists, create one. If a Draft PR exists, mark it ready for review. **Before writing the PR description**, consult the team's captured operating conventions for the expected PR description format — what sections are required, how linked work items should be referenced, what should and sh