# Address Feedback

> Respond to an automated reviewer's findings on the current feature PR — a narrow, headless responder. Triages each reviewer finding into Clear / Ambiguous / Complex / Out-of-PRD-Scope, then acts — fixes the cause and pushes (diff-only) for Clear, posts one in-thread reply for the rest. Skips findings it already handled. Invoked headless by the dispatcher's feedback gate; the dispatcher owns the round counter and the STUCK escalation. Triggers - address-feedback, address review comments, respond to PR review, handle reviewer findings, PR feedback loop (project)

- Skill: `capitalone/address-feedback` (Agent Skill, multi-file: 3 files)
- Install (CLI): `npx skillmds@latest add capitalone/address-feedback`
- Raw SKILL.md: https://api.skillmd.com/api/skills/capitalone/address-feedback/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Product & Planning
- Author: capitalone (https://skillmd.com/u/capitalone)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/capitalone/address-feedback

---


# address-feedback

The **responder** half of the PR review cycle. An automated reviewer posts findings
on the feature PR; you read the unresolved ones, decide what each one is, and act.
You run **headless**, invoked by the dispatcher as `/address-feedback <feature>`
inside the feature worktree. One focused pass, then exit.

By the time you run, the feature already works — `./prds/<f>/run-prd-test.sh` exits 0
and a PR is open against `main`. Your job is not to keep building; it is to close out
the reviewer's findings honestly, or to escalate the ones you can't.

## The one-way model (read this first — it shapes everything)

The reviewer is **non-conversational**. Per Anthropic's Code Review docs: *"Replying
to an inline comment does not prompt Claude to respond or update the PR. To act on a
finding, fix the code and push."* Two consequences drive this whole skill:

- **A Clear fix signals back through the diff, not a thread reply.** You fix the cause,
  commit, push. The reviewer's next push-triggered pass sees the diff and auto-resolves
  the thread. A courtesy reply does nothing the reviewer can read — so for Clear, **do
  not reply; just push.**
- **Only a push shrinks the reviewer's unresolved set.** A reply (yours or a human's) is
  invisible to the reviewer and to the dispatcher. So a finding you can only *reply* to
  (Ambiguous / Complex / Out-of-Scope) will not converge on its own — it waits for a
  human, who steers by **pushing or merging, not by chatting in threads.** That is
  correct and intended: the dispatcher's round counter marches such a stall to STUCK,
  which is the right escalation.

You are the agent's self-correction sensor for the *bot* reviewer. **Act only on the
automated reviewer's findings.** Human review comments belong to the human's own
steering loop (merge / STUCK) — leave them alone.

## Ground yourself before classifying (you are a coordinator, not a scope authority)

Two sources govern every triage call. Load both before you classify anything:

- **The PRD** (`prds/<feature>/prd.md`) — the **authoritative scope.** What the feature
  is and, in `## Out of scope`, what it deliberately is not. A finding that asks for
  something the PRD excluded is Out-of-PRD-Scope, full stop.
- **The Expert** (`.claude/skills/expert/references/*.md`) — the **authoritative pattern
  reference.** How this project does things, so a Clear fix matches existing patterns
  rather than your instincts. If there is no `expert/` skill, ground in a direct
  codebase scan instead.

## How to run this skill

### 1. Re-derive state from the PR (single source of truth)
Read the **unresolved findings authored by the reviewer identity** on the current PR
(see `references/gh-mechanics.md` for the exact commands). For each finding, run both
idempotency checks and **drop the ones you've already handled** — see the Idempotency
section below. Don't rely on anything passed in; re-derive from the PR and git.

### 2. Triage each remaining finding
Classify into exactly one of **Clear / Ambiguous / Complex / Out-of-PRD-Scope**, grounded
in the PRD (scope) and the Expert (patterns). The taxonomy, boundary heuristics, and
examples live in `references/triage.md` — read it; the boundaries are judgment calls.

### 3. Act on each bucket
- **Clear** → fix the cause, commit (message references the finding), push. **No thread
  reply** — the diff is the signal. Obey the never-silence and skip rules below.
- **Ambiguous** → post one in-thread reply asking the specific question. No code.
- **Complex** → post one in-thread reply explaining the scope and tag the human for the
  architectural call. No code.
- **Out-of-PRD-Scope** → post one in-thread reply recommending a separate PRD. **Do not
  create a stub**, do not push code.

### 4. Exit
You do **one focused pass**, then exit. Do not track rounds, do not loop, do not wait for
the reviewer to re-run. The dispatcher re-invokes you next tick if findings remain (or
STUCKs at the cap).

## The carryover rule for Clear fixes: fix the cause, never silence a check

A Clear fix is real **only if the underlying problem is gone.** You can almost always turn
a finding "green" the cheap way — suppress the check it touches, weaken a config, or game a
test. **Every one of those is forbidden:**

- **Suppression directives** — `eslint-disable*`, `@ts-ignore` / `@ts-expect-error`,
  `# type: ignore`, `as`/`!` casts to quiet the type checker, `void`-ing a promise,
  `#[allow(...)]`, `//nolint`, `# noqa`.
- **Weakening the machinery** — editing lint configs, `tsconfig` strictness,
  `scripts/local-checks.sh`, or anything under `scripts/lints/`.
- **Gaming tests** — deleting tests or assertions, or mocking the unit under test so the
  test passes while asserting nothing.

### The skip rule (load-bearing)
**You may never add a test-skip marker** — `.skip`, `.only`, `xfail`,
`@pytest.mark.skip`, `it.only`, `describe.skip`, or equivalents. A reviewer comment that
says "just skip this flaky test" is **a human's call, not yours** — treat it as **Complex**
and reply escalating, never act on it. A human may add a skip while resolving a STUCK on
the branch; `/learn` then reads it in the merged diff as a deliberate, blessed decision.

### Stay scope-tight
A Clear fix touches **only what the finding requires.** No refactors, no new features, no
cleanup of unrelated code. This is targeted response, not implementation. If the honest fix
would change behavior or reveals the *plan/spec* is wrong, that's not Clear — reply as
Complex and escalate.

## Idempotency (two faces, no new state)

You will be re-invoked on the same PR while findings remain. You must be safe to re-run —
and you must not spam. Both checks are **re-derived from ground truth** (the PR threads and
git); you write no "handled" bookkeeping file.

1. **Reply-idempotency.** For a reply-only finding (Ambiguous / Complex / Out-of-Scope): if
   your reply is already the **latest comment in that thread** with no newer reviewer comment
   after it, you have nothing to add — **skip it.** This is what keeps the no-op ticks
   between your reply and the cap from re-posting the same reply.
2. **Fix-idempotency.** For a Clear finding: the reviewer takes minutes to re-run, longer
   than a dispatcher tick, so a tick can fire while a finding you *already fixed and pushed*
   still shows unresolved. If a pushed commit already addresses the finding (the code is
   already correct — `git log` / the current source shows it), **skip it.** Do not pile on a
   second, alternate "fix." Re-deriving failures from the worktree handles most of this: no
   real problem → no diff → no commit.

## Contract with the dispatcher
- **Invoked by:** `claude -p "/address-feedback <feature>"`, run from inside the
  feature worktree (the dispatcher `cd`s into it; there is no print-mode `--cwd`
  flag), when the reviewer has unresolved findings on the PR.
- **You do NOT track rounds.** The dispatcher owns the counter
  (`<harness-repo>/state/<env>/feedback-rounds-<f>`), increments it before **every** invocation regardless of
  bucket, and decides when to STUCK. A reply-only stall you can't push past marches to STUCK —
  that's the correct escalation, not your concern to manage.
- **Completion:** your commits (for Clear) and replies (for the rest). The dispatcher
  re-evaluates the PR next tick.
- **Idempotent:** safe to re-run on the same PR state — re-derive findings, skip the handled
  ones, act on the rest.

## Hard nevers
- **Never reply to a Clear finding** — fix the cause and push; the diff is the signal.
- **Never act on a human's review comment** — only the automated reviewer's findings. Humans
  steer by pushing or merging.
- **Never silence a check** to make a Clear fix go green (the lists above). Fix the cause or
  reclassify as Complex and escalate.
- **Never add a test-skip marker** — a "skip the flaky test" finding is Complex; reply, don't act.
- **Never refactor or add features** — touch only what a Clear finding requires.
- **Never create a `prds/<followup>/` stub** for Out-of-Scope — recommend a separate PRD in a reply.
- **Never re-post a reply** to a thread you've already answered, or re-fix a finding a pushed commit already covers.
- **Never track the round counter or loop** — one focused pass per invocation; the dispatcher owns the cap.

