Skip to content

feat(skills/developing-a-feature): add post-review change choreography for history-sharing PRs#25

Merged
sourcehawk merged 1 commit into
mainfrom
feat/post-review-choreography
Jul 10, 2026
Merged

feat(skills/developing-a-feature): add post-review change choreography for history-sharing PRs#25
sourcehawk merged 1 commit into
mainfrom
feat/post-review-choreography

Conversation

@sourcehawk

Copy link
Copy Markdown
Owner

Problem

When a multi-PR feature has several open PRs whose branches share history (stacked sub-PRs, concurrent feature-branch sub-PRs, wave N+1 off wave N) and a human reviews them live, the plugin had no guidance for where a review fix lands or what it must drag along. Observed failures in a real orchestration session:

  • A fix for code introduced by an ancestor PR was committed on the descendant branch where the reviewer happened to comment, polluting that PR's diff and leaving the ancestor to merge with the defect intact.
  • Scripted state-file table updates used guarded substitutions that silently no-oped on a format mismatch; the resulting stale head SHAs (three pushes behind) surfaced only when the user asked "wrong branch?".
  • Review decisions propagated to code but not consistently to the spec, plan, issue bodies, and sibling PR bodies, forcing a large wrap-up reconciliation sweep.

Change

skills/developing-a-feature/SKILL.md gains one section (Review-driven changes while several open PRs share history) with three rules — fix where the code lives (establish ownership via git log -S before editing), propagate by merging forward (never rebase under live review), propagate the decision (spec/plan, state file, issue bodies via writing-github-issues Step 2D, stale PR bodies) — plus a positive recipe for scripted state-file edits (assert every row matched, abort without writing, remote-read SHAs at write time). One matching anti-pattern and three red-flag rows.

Verification (writing-skills RED → GREEN)

  • RED: a baseline subagent given the unmodified skill and the state-file scenario produced a guarded sed -i whose non-match no-ops silently, backstopped only by an eyeball diff — the exact observed failure. The fix-at-origin failure is documented from the live session (narration-level baselines passed, so that side ships as compact rules + red-flag counters rather than teaching prose, per writing-skills form-matching).
  • GREEN: with the edit in place, the same state-file scenario produced an assert-then-write updater (row-keyed, remote SHAs, abort-on-anomaly, prints old → new) citing the new block under speed pressure; the stacked-PR scenario stayed fix-at-origin/merge-forward and walked the full decision-propagation list.

Copilot AI review requested due to automatic review settings July 4, 2026 03:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds explicit choreography to the developing-a-feature skill for handling review-driven fixes when multiple open PRs share git history (stacked / concurrent sub-PRs), so fixes land on the correct branch, propagate safely, and the accompanying planning/state artifacts stay consistent.

Changes:

  • Adds a new section with rules for “fix at origin”, “merge forward (no rebase)”, and “propagate the decision” across spec/plan/state/issues/PR bodies.
  • Introduces a concrete “assert, then write” recipe for scripted state-file PR-table updates (row-keyed assertions + remote head SHA reads).
  • Extends the Anti-patterns list and Red flags table with matching entries to reinforce the new guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sourcehawk sourcehawk merged commit cd6f519 into main Jul 10, 2026
1 check passed
@sourcehawk sourcehawk deleted the feat/post-review-choreography branch July 10, 2026 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants