review-change

Files (1)

• SKILL.md

SKILL.md (first lines)

# review-change

You are the orchestrator. Review only: do not apply fixes or run reviewed code.
Read `docs/report-contract.md` from the extension directory. Schema v2 is required.
Relative references in this skill are relative to that directory. Node helpers
belong to the extension, not the target.

You freeze input, collect history, and map coverage. You do **not** play the
reviewer, challenger, or judge: each of those steps runs in a fresh `pi -p`
process launched by `bin/run-phase.mjs`, which injects the `agents/*.md` role pack,
lens rules, and your notes into that child's context. pi has no subagents; a
process per step keeps the phases independent (ADR 0006). All (lens, slice)
tasks share one pool in `bin/run-lenses.sh`, one slot per lens; a free slot takes
the next task of any lens. No `Agent` tool.

Create the work dir first, in system temp and never inside the target:
`WORK=$(mktemp -d -t adversarial-review-XXXXXX)`. Task files and child logs live there.

## 0 — Resolve and freeze the input

Parse target, optional base, `--lenses`, and `--verify`. An empty target means the
current branch; report that choice. Unrecognized prose is **not** a branch or a
license to turn a focused allegation into a full review. Stop with usage instead.
An explicit `--verify` selects finding verification; otherwise scope is the full
change under the selected lenses.

Resolve the absolute Git root as `TARGET_DIR`. Record UTC time, repository, target,
PR number (or null), full `base_sha`/`head_sha`, and the input's intent:

- **PR URL/number:** use `gh pr view` for title, body, head/base names and OIDs.
  Freeze those OIDs; use their three-dot diff when available locally. A live
  `gh pr diff` is usable only if PR OIDs checked before/after it are unchanged.
  GitHub's actual base wins over a stale local `main`. If `gh` fails, stop.
- **Branch:** resolve its commit. Honor an explicit base. Without one, use the
  matching PR's actual base OID; if there is no PR, resolve hosted `main` through
  `gh api`. If freshness cannot be established, request an explicit base rather
  than silently treating local `main` as current. Record commit messages as intent
  and include matching PR title/body when available.
- **Patch:** read it, compute SHA-256, record absolute `patch_path`, use target
  `patch:<digest>` and null Git SHAs. Intent is unknown: solution-fit is
  correctness-only unless the user supplies requirements. State that context is
  limited to what can be matched to the patch, not an arbitrary checkout.

Save the diff as `$WORK/change.diff`. Empty diff: say so and stop.
Use a clean checkout at the frozen head for source context. Read-only Git metadata
checks must confirm it matches; do not checkout, stash, reset, fetch, or modify
refs to make it match. If objects/context are unavailable or the working tree
changes, mark `snapshot_consistent: false` and deliver INCOMPLETE, not approval.
Do not mix current working-tree source with a different reviewed revision.

### Requirements

With JIRA credentials, extract ticket keys from the branch, PR, and commit subjects;
fetch API v2 summary/description/status/type/parent. Never print credentials.
Prioritize the primary story and linked implementation spec; separate inherited
cross-repository requirements from work owned by this PR. A PR number/issue text
is not evidence that a requirement has shipped.

No credentials/no key/fetch failure: record the limitation without stopping.
Exact-head existing CI logs may be read with `gh` to settle test claims. Do not use
old PR-description pass counts or coverage from another SHA as current evidence.
No repository tests/builds or live service/cloud calls are allowed in this skill.

## 1 — Load the cumulative ledger

Before discovery, collect matching history:

```bash
node <extension>/bin/report-tools.mjs collect <TARGET_DIR> <target> [pr-number] > "$WORK/prior.json"
```

Read the resulting ledger, source paths, and relevant prior evidence. Preserve the
collector's `prior_reports` digests. Every known ID must survive, either directly
or as an evidence-backed alias. Do not restart F1 numbering; reuse stable IDs.
Legacy IDs are supplied by the collector. Reconcile wording variants explicitly,
not by guessing that findings in the same file are identical.

Assign each prior issue one of: open, fixed_verified, disproved, deferred,
not_rechecked. Use the closure requirements in `docs/report-contract.md`.
Missing findings are **not** fixed findings. A clean recent report cannot erase
older open concerns. Preserve first-seen date, confidence, severity, and evidence
when a finding is not rechecked. Separate severity from certainty.

For `--verify id,...`, resolve IDs against this ledger (accept a legacy report-local
ID only when the user identifies its source report). Verify only those findings;
carry every other finding as not_rechecked with its prior disposition. No overall
implementation/solution-fit/SHIP verdict is allowed in verification mode.

## 2 — Mechanical impact and behavior coverage

Extract changed exported symbols, routes, flags, data contracts, and action names.
Build the reverse importer/caller map using the repo's indexed analysis or ast-grep
(fallback grep). Record test references separately. Restrict results to TARGET_DIR;
parent-workspace/sibling-repo hits and stale excerpts are not trustworthy coverage.
Record gaps; do not infer no callers or no tests from missing graph results.

Then build a compact invariant/behavior matrix BEFORE proposing fixes:

- Entry point and reachable mode/flag/permission combinations.
- Shared mutable state, **all writers**, and the final consumer/business outcome.
- Success, rejection, cancellation, supersession, replacement, unmount, and
  never-settling work where relevant. Check both safety and liveness.
- Downstream effects: payload sent, state committed, user-visible result, persisted
  operation. Do not stop at an intermediate request if a later callback books.
- Test/assertion or exact-head CI evidence that discriminates the behavior.

For each row record criticality and verified / unresolved / not_inspected /
not_applicable, with evidence or a concrete next check. Do not enumerate impossible
Cartesian combinations; prove render/caller eligibility before claiming a UI race.
An inspected hunk or a listed test name does not by itself verify a behavior.

All diff sections remain in scope. For large changes, group into coherent behavior
slices and explicitly inventory what each slice covers. If the budget is too small,
mark the remaining critical rows uninspected; do not redefine the change as reviewed.

## 3 — Select lenses and discover, then challenge

Read every `lenses/*/lens.md` plus the repo overlay (`run-phase.mjs` appends
same-named overlay rules to each child's context).

From gbencke/dotfiles, MIT.

More from gbencke/dotfiles