Self Review

---
name: self-review
description: >
  Gate-wired review pass of the current feature branch: Discovery Rigor
  fan-out, multi-pass validation, act-then-review dispositions per the finding
  categorization, full audit record (lens-coverage table, four bucket tables,
  declined log, pending-sign-off checklist). Standalone runs push and open or
  update a draft PR; pass --nested to stay local and return the audit record
  to the invoking skill.
argument-hint: "[--nested]"
---

# /self-review

One complete review pass of the feature branch against its base, wired into
planwright's act-then-review autonomy gate (REQ-E2.1, D-12). `/polish`
iterates it to convergence; this skill is the single pass.

## Doctrine

This skill is procedure, not doctrine. Resolve the rule docs in the manifest
below via the rule-doc resolution convention
(`scripts/resolve-rule-doc.sh <doc-name>` under the resolved planwright root,
or the documented `PLANWRIGHT_ROOT`/`CLAUDE_PLUGIN_ROOT` chain); their
definitions govern wherever this skill names a concept. Issue one plain command
per Bash call (`doctrine/plugin-script-invocation.md`).

If a rule doc does not resolve, halt with a clear message naming the missing
doc and the resolution chain consulted.

Doctrine manifest (per `doctrine/instruction-hygiene.md`; `run-start` docs
load before work begins, `point-of-use` at the named step):

Doctrine: run-start discovery-rigor
Doctrine: run-start validation-rigor
Doctrine: run-start finding-categorization
Doctrine: run-start gate-wiring
Doctrine: point-of-use research-rigor (the Validation step, where research triggers fire)
Doctrine: point-of-use refactor-instinct (review mode, at the Discovery refactor filter)
Doctrine: run-start security-posture (artifact data-hygiene)
Doctrine: run-start proportionality (declared scoping)

## Invocation modes

Read the literal flag `--nested` from `$ARGUMENTS` at the start of the run:

- **Standalone** (no flag): the pass ends by pushing the branch and creating
  or updating a draft PR carrying the audit record (see "Publishing the audit
  record").
- **Nested** (`--nested`): another planwright skill (typically `/polish`)
  invoked this pass and owns everything after it. Skip push and PR handling;
  hand the audit record back to the invoking skill. It may pass its loop
  ledger of already-dispositioned findings: a re-discovered finding that
  already carries a disposition is reported in the audit record but never
  re-routed, re-applied, or re-paused. One exception: surface a
  re-discovered finding whose ledger disposition is any on-branch
  application (applied, resolved, or applied pending sign-off) prominently
  in the handed-back record; it signals the fix did not hold and feeds the
  invoking skill's loop detection.

Nested invocation is in-session skill composition (REQ-E2.2, D-13): the pass
runs in the invoking skill's session and context, and hooks fire once per
actual tool call, not once per skill layer. Record the resolved mode in the
pass summary.

## Pre-flight

1. **Resolve the doctrine docs** (above). Halt with a clear message on a
   resolution failure.
2. **Identify the base and capture the diff.** Fetch first, then diff against
   the remote-tracking base: `git fetch origin` and
   `git diff origin/main...HEAD` (substitute the repository's actual default
   branch). Fall back to the local base only when no remote is configured
   (REQ-K1.7), since a stale local base inflates the diff with already-merged
   commits. In nested mode, skip the fetch and reuse the base the invoking
   skill recorded at its own pre-flight; the parent owns remote interaction.
   Not a git repository, or no commits to diff: surface a clear message and
   stop; there is nothing to review.
3. **Require a clean working tree.** The gate's commit discipline needs
   unambiguous boundaries. If `git status --porcelain` is non-empty, surface
   the dirty state and ask before proceeding (dispatched or unattended: record the
   unit to `tasks.md` Awaiting input and end the step, the pause protocol's
   dispatched arm); never stash or discard.
4. **Run the project's tooling once.** Whatever the project ships: linters,
   formatters, type-checkers, static analyzers, security scanners. Discover
   them via the project's task runner and config files, CI workflow
   definitions, and the tool-discovery summary when a SessionStart hook has
   injected one. Capture the output; it is shared input for every lens. A
   tool whose runner itself errors out (as opposed to reporting rule
   violations) is surfaced in the pass summary as degraded grounding for the
   lenses that rely on it; never treat a crashed tool as a clean pass.
5. **Detect the active kickoff brief.** Walk in order, stopping at the first
   unambiguous match: the branch convention `planwright/<spec>/task-<ids>`
   names the spec, so the brief is `specs/<spec>/kickoff-brief.md` (validate
   the parsed `<spec>` segment against the spec-identifier discipline,
   `^[a-z0-9][a-z0-9-]*$` and at most 64 characters per REQ-A1.8, before
   forming the path; a failing segment is treated as no match, never
   interpolated); a branch-named spec whose own `kickoff-brief.md` is absent
   means the brief is absent for this pass, never a fall-through to another
   spec's brief, and so does a flight branch `planwright/flight/<id>`, which
   names no spec. Otherwise resolve through the status render
   (`scripts/spec-status.sh` / `mise run status`; invariant-tasks D-6),
   accepting a `specs/*/` bundle whose derived status is Ready or Active — so
   a format-version-2 bundle with work in flight (stored Ready, derived
   Active) resolves, not only a stored-`Active` v1 spec. When exactly one such
   candidate exists, take its sibling `kickoff-brief.md`. A render error, zero
   candidates, or multiple Ready-or-Active candidates are not an unambiguous
   match: degrade to the existing arm — ask when attended, proceed brief-less
   when unattended or dispatched (D-2). With
   no active brief, the Agent-resolvable bucket is unavailable for this pass
   (its predicate requires brief alignment); record that and proceed with
   the remaining buckets.

## Discovery

Apply the `discovery-rigor` doc against the diff:

- **Fan out by default.** Spawn one read-only sub-agent per canonical lens
  for any non-trivial diff, each with the diff slice, the shared tooling
  output, and a single-lens brief that forbids severity pruning. Walk the
  lenses inline only when the diff is small and narrow (a few files, mostly
  prose or config, a few hundred changed lines at most); inline walking
  waives no other invariant. Declare which path was taken.
- **Merge and dedupe.** A finding hitting two lenses gets one row with both
  labels.
- **Filter refactor flags in review mode** per `refactor-instinct`: anchored
  in tool output or made worse by this branch, otherwise dropped. This bullet
  is the doc's only read site, which is what its `point-of-use` classification
  rests on; a second, earlier read site needs the manifest revisited.
- **Hold the Documentation lens to `discovery-rigor`'s four defect classes**:
  prose the diff falsifies, documentation missing for behaviour the diff
  introduces, a documentation guard's violation, and an interpretation fork.
  Merely improvable prose is not a finding, and a `none` row that says so is
  coverage rather than a gap.
- **Emit the canonical lens-coverage table** before any per-finding output:
  one row per lens, empty lenses as `none` or `n/a` with a one-line reason.
- **Run the mandatory self-critique pass**: assume the list is incomplete,
  re-scan for what is under-represented, add what surfaces.

## Validation

Apply the `validation-rigor` doc to every candidate finding. Declared scoping
(per `proportionality`): the full three passes are mandatory for any finding
that could be **applied on the branch** in this pass, which under
act-then-review means Auto-applicable, Agent-resolvable, and Needs-sign-off
candidates alike. A soft-floor spot-check (drop clear false positives) is
sufficient for findings that will only be declined or queued as judgment
forks, since the human finishes validation when deciding those. Findings
whose passes do not converge are never applied; they are dropped, declined
with the divergence recorded, or routed to Needs human judgment per the
categorization.

When a finding turns on an external fact (an API contract, spec compliance, a
deprecated pattern or security claim, library behavior), the outside-in pass
reads `research-rigor` point-of-use here — source hierarchy, recency
discipline, antipattern check.

## Routing and dispositions

Route every validated finding through the `gate-wiring` doc's routing order:
zone screen first, then bucket assignment per the `finding-categorization`
predicates, then disposition by bucket. The wiring doc governs the mechanics
this skill executes. Every artifact the pass emits passes `security-posture`'s
artifact data-hygiene first: commit bodies, audit rows, the PR body,
observation fragments, and the lens-coverage table Discovery has already
produced, whose Notes cells quote tooling output.

- A finding whose fix edits **only prose** is classed on
  `finding-categorization`'s *Prose findings* axis before it is routed:
  expression-only when no normative statement changes meaning, meaning-class
  otherwise.
- Auto-applicable and Agent-resolvable items are applied or resolved with
  their audit and evidence rows, and committed per the wiring doc's commit
  discipline; their fixes get solution validation per `validation-rigor`.
- Attended or unattended, a Needs-sign-off fix is committed on the branch,
  never asked about first, per that same discipline (its
  `Planwright-Sign-Off` trailer via
  `scripts/planwright-commit-trailers.sh --sign-off`), and entered in the
  checklist.
- Needs-human-judgment candidates climb the resolution ladder; every
  consulted rung is recorded. Only irreducible forks queue, with bespoke
  options.
- Declined-with-rationale is available at any step after validation;
  rationale rows are mandatory.
- A hard-disqualifier zone or a blocking irreducible fork triggers the pause
  protocol; nothing else interrupts the pass.

**Findings whose fix edits a signed bundle** carry the three obligations the
meta-spec's writer prose states (`doctrine/spec-format.md`, *Sign-off records
and content anchors*), which governs here. Before the first edit to each Ready or
Active bundle's anchored content, run the **stale-anchor pre-flight**: recompute with
the brief's most recent recorded command; a mismatch, an absent or unparseable
entry, and a failed recompute each block that edit and surface instead of
editing on top of it (findings elsewhere in the diff keep routing normally).
A blocked finding is never left undispositioned: it queues as an irreducible
fork whose route is the anchor repair. An **expression-only** fix then lands
the edit, its dated Changelog entry, and the marked
`Class: expression-only` self-re-anchor entry citing it in **one
commit**. A **meaning-class** fix is never applied: leave the bundle untouched
and queue it as an irreducible fork naming a `/spec-kickoff` delta
re-walkthrough as the route.

If any applied fix breaks the wider project suite and the breakage cannot be
resolved within the finding's own scope, revert that finding's change (new
commit, never history rewrite) and surface the failure in the pass summary.
The reverted finding's terminal disposition is declined-with-rationale
("fix attempted, broke the wider suite, reverted"), recorded in the
declined log, where it stays visible and re-raisable without re-entering the
routing order.

## The audit record

The pass produces, **artifact-side**, in this order (the wiring doc's loop-end
handoff, extended with the lens-coverage table at the front and the pass
summary at the end):

1. The lens-coverage table.
2. In the wiring doc's formats: the four bucket tables, the declined log, and
   the pending-sign-off checklist regenerated from sign-off trailers ahead of
   the base.
3. Queued irreducible forks with their bespoke options.
4. The pass summary: resolved mode, base used, tooling and wider-suite
   results, and any reverts or surfaced failures.

The **turn** gets the wiring's projection: counts, each pending sign-off and
each fork; standalone, only forks are questions. Print each trailered
commit's SHA and the command `git log --format='%h
%(trailers:key=Planwright-Sign-Off,key=Planwright-Sign-Off-Rejected,separator=%x20)
%s' <base>..HEAD`.
The approval act, never the ready flip, signs off
pending-sign-off items; reject one before it by its printed recipe.

## Publishing the audit record (standalone only)

Publishing is the pass's final action: the Observations and Maintenance
steps below run first, so their chore commits land before the push.

1. **Push:** `git push origin <branch>` (with `-u` on first push). Never
   force-push. On push or authentication failure, degrade gracefully
   (REQ-K1.6, REQ-K1.7): the local work is committed; surface what failed
   and stop.
2. **Draft PR:** if a PR already exists for the branch, update its body;
   otherwise `gh pr create --draft` with an explicit `--title` and `--body`
   (headless `gh` prompts or fails without them). Assemble the body per the
   **PR-body assembly** section of the `gate-wiring` doctrine, the single
   normative home for the layout (D-2); the collapsed audit record is this
   pass's own (the sequence in **The audit record** above). The PR is always a
   draft; never mark it ready and never merge (the draft→ready flip is the
   human's call).

## Observations

When the repository has adopted planwright (a `specs/` directory with at
least one spec bundle exists), record anything noticed during the pass that
is outside the branch's scope (complexity growth, outdated patterns, tooling
gaps, doctrine gaps) as one fragment per observation through the shared
helper: `scripts/obs-record.sh --slug <topic> --scope <repo> --text
'<observation>'` (resolved under the planwright root; it writes one fragment
file under the host repo's `specs/_observations/entries/`). Commit the
fragment within the pass (the action commit, or its own chore commit when
nothing else landed); never leave the tree dirty at a pass boundary, and
surface a non-zero helper exit rather than silently dropping the
observation. Do not act on observations during the pass; they are seed
material for `/spec-draft` (REQ-E2.1, REQ-H1.6).

## Maintenance

After the pass completes (or halts), compare these instructions against the
resolved doctrine docs listed above (REQ-B3.2, D-42). If a concept this skill
names has changed meaning, gained or lost a step, or moved between docs,
record a drift observation through the shared helper (`scripts/obs-record.sh
--slug skill-drift --scope <repo> --text 'skill-drift(self-review): <what>'`
— the entry text keeps the `skill-drift(...)` prefix; in repositories
without `specs/`, surface the drift to the user instead of recording it),
commit the fragment as its own chore commit, tell the user what drifted, and
surface a non-zero helper exit rather than silently dropping the observation.
Do not edit this skill or the doctrine docs to resolve the drift; `/spec-draft`
owns folding drift into spec amendments.