Skip to content

Post-review next steps don't match the reviewer's role #1014

Description

@williamthorsen

Problem

_partials/next-steps-after-review.md offers actions a reviewer never performs — design-and-plan (three separate routings) and orchestrate-dev — while omitting the ones the workflow actually uses. A reviewer surfaces findings; the author disposes of them. Re-designing, re-planning, and orchestrating belong to neither role.

Both purpose-built skills exist and nothing routes to either: respond-to-review ("bridges the gap between receiving a code review and implementing fixes") is reachable only by typing it, and bb-pr-inline-comment ("during code review, to leave line-level feedback on a pull request") is referenced by review-bb-pr only for its auth mechanism and URL parser.

The visible symptom is a name collision: "Update ticket" appears in two sub-blocks with different remedies — Deviations routes it to design-and-plan, Source divergence to align-ticket-with-implementation — and the partial's own combined-output example renders both in one turn with the same name, emoji, marker, and job description.

The menu also serves two entry points with one option set. review-pr invokes review-branch's process from step 4, so both render the same block — but a PR's author is typically someone else, while a local branch's code is ours.

A second defect blocks the routing fix: align-ticket-with-implementation emits ## Description / ### Issues / ## Context, while design-and-plan and create-ticket emit the shared ticket-skeleton.md shape. Routing to it today would hand back a restructured ticket.

Context

The misrouting dates to #331 (2026-03-17), which created the Deviations sub-block and routed "Update ticket" to design-and-plan. The purpose-built skill already existed — as ex-post-facto, since the agents workspace's first commit (2026-03-01) — but its name didn't say what it did, so it wasn't found. #628 renamed it to align-ticket-with-implementation and routed the new Source-divergence sub-block to it correctly, which is what made the older routing visibly wrong.

The finding scheme already encodes the role model. Its actionability gate: "Every finding must hand the author a concrete decision they can act on in the change under review."

Proposed solution

Rebuild the menu on the reviewer/author roles, and split the Findings options by entry point.

review-pr — the author is typically someone else:

Actionable findings:
1. 📋 ■■□ Post findings on the PR
2. 🚀 ■□□ Implement directly

review-branch — the code is ours:

Actionable findings:
1. 📋 ■■□ Ask the author to address the findings
2. 📋🔍 ■□□ Wait for the author to address the findings, then `review-branch`
3. 🚀 ■□□ Implement directly

If the author is an agent, run `respond-to-review` in that session.

Both "Update ticket" options route to align-ticket-with-implementation and differentiate by name — "Update the acceptance criteria" (Deviations) and "Update the stale ticket" (Source divergence) — so the collision dissolves rather than being managed. Source divergence loses 🧠 Revisit design from all cases; case 4 renders marker-free, since the reviewer can't tell whether the code or the specs are wrong.

Name a skill in the rendered output only when the user carries the invocation across a session boundary — clearing context, walking to another session, waiting for someone else. Otherwise the agent runs it and the skill lives in the agent-facing Options table.

align-ticket-with-implementation adopts ticket-skeleton.md, reading ## Proposed solution as the approach the branch took.

On GitHub, review-pr's "Post findings on the PR" needs a mechanism review-gh-pr doesn't have; #1018 tracks that gap and does not block this ticket.

Acceptance criteria

Must have

  • No next-steps option routes to design-and-plan or orchestrate-dev.
  • A single next-steps turn never offers two identically named options.
  • review-pr and review-branch render different Actionable-findings options.
  • A rendered option names a skill only when the user must carry the invocation across a session boundary.
  • align-ticket-with-implementation emits the shared ticket skeleton, so amending a ticket authored by design-and-plan or create-ticket preserves its section structure.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions