enhance(#4107): sequence external review after internal fixes (#4206)

* enhance(#4107): sequence external review after internal fixes

Teach the planner to finish internal review and accepted fixes before opening a PR known to trigger automatic external review. If an open-time property exists, re-check it immediately before opening with nothing intervening; post-open CI, review, changeset, and tracking work may follow.

Emitted-Drift-Ack-Growth: gsd-planner.md — issue #4107 adds the review-before-publish ordering rule

* chore(#4107): add PR #11 changeset

* chore(changeset): link upstream PR 4206

* fix(#4107): ground external-review terms and tighten ordering test

Addresses trek-e review on PR #4206:
- Ground 'known automatic external review' and 'open-time property' with
  concrete anchors (CodeRabbit App / .coderabbit.yaml, not-behind-base).
- Suffix the antipatterns heading with (#4107), matching sibling sections.
- Replace vacuous negative assertion with inverted-order fixtures that
  prove the ordering regexes reject bad phrasing, not just co-occurrence.

* fix(#4107): make directionality fixtures genuinely adversarial

agy (gemini-3.8-flash-high) adversarial review found the two negative
fixtures added in 584ec1cda were vacuous: they proved the ordering regexes
require certain keywords, not that they reject inverted order — the bad
strings simply omitted required tokens rather than reordering them.

- Rebuild both fixtures to contain every required token, reordered/negated,
  so a real reordering would still slip past a weaker regex.
- Drop the unsupported 'changeset' mention from the Wave 4+ antipatterns
  example — gsd-core/workflows/ship.md never references changeset work,
  so naming it here implied a step this rule doesn't actually govern.

* fix(#4107): make the full review-then-fix-then-open sequence explicit

CodeRabbit (fork PR #11) flagged that the planner prose only ordered
accepted fixes before PR open, without explicitly naming 'run internal
review' as its own earlier step, and that no fixture tested the planner
text's own wording for inversion (only the antipatterns example had one).

- Prose now reads 'run internal review and apply the accepted
  internal-review fixes before the final open'.
- Added a planner-text-specific inverted-order fixture alongside the
  existing antipatterns-example one.

---------

Co-authored-by: Test <test@test.com>
Co-authored-by: Tom Boucher <trekkie@nomorestars.com>

.changeset/eager-tunas-gather.md

@@ -0,0 +1,7 @@
+---
+type: Changed
+pr: 4206
+---
+**Planner wave assignment now sequences automatic external review after internal fixes** — phases with internal review lanes defer PR creation until accepted fixes land and re-check open-time properties immediately before opening.
+
+<!-- docs-exempt: this changes internal planner guidance only; no command, configuration, schema, or user-facing documentation contract changes -->

agents/gsd-planner.md

@@ -756,6 +756,8 @@ for each plan B in plan_order:
 
 **Rule:** Same-wave plans must have zero `files_modified`/`files_deleted` overlap. After assigning waves, scan each wave; if any file appears in 2+ plans, bump the later plan to the next wave and repeat.
 
+**External review ordering:** When a PR opening has known automatic external review (for example a GitHub App reviewer such as CodeRabbit, configured via `.coderabbit.yaml`, which reviews automatically on PR open) and the plan includes internal review lanes, run internal review and apply the accepted internal-review fixes before the final open. If an open-time property exists (for example a not-behind-base check that must legitimately be measured at PR-open instant), re-check it immediately before opening, with nothing intervening; post-open CI, review, and tracking may follow. Examples: @gsd-core/references/planner-antipatterns.md ("External Review Before PR Open (#4107)").
+
 Non-file coupling: @~/.claude/gsd-core/references/planner-coupling.md
 </step>
 

gsd-core/references/planner-antipatterns.md

@@ -228,3 +228,28 @@ test -f src/i18n/en.json && test -f src/i18n/de.json || { echo "missing input fi
 ```
 
 **When `|| echo "default"` is acceptable:** only when absence is semantically the default AND the result is NOT used in a comparison that should detect absence.
+
+## External Review Before PR Open (#4107)
+
+Apply this ordering only when opening the PR is known to trigger automatic external review and the plan also has internal review lanes.
+
+**Bad:**
+
+```text
+Wave 1: Open PR; automatic external review starts
+Wave 2: Run internal review
+Wave 3: Apply accepted fixes
+```
+
+The external reviewer spends its first pass on a diff the plan already expects to change.
+
+**Good:**
+
+```text
+Wave 1: Run internal review
+Wave 2: Apply accepted fixes
+Wave 3: If applicable, re-check the open-time property; then immediately open PR
+Wave 4+: Run post-open CI, external review, and tracking work
+```
+
+Nothing may intervene between an applicable re-check and the open. Post-open work may follow; "immediately" constrains only that gap. Opening-time properties do not justify an early PR.

tests/prompt-thinning.test.cjs

@@ -17,6 +17,7 @@ const { test, describe } = require('node:test');
 const assert = require('node:assert/strict');
 const fs = require('fs');
 const path = require('path');
+const { collectSection } = require('../gsd-core/bin/lib/markdown-sectionizer.cjs');
 
 const EXECUTE_PHASE = path.join(__dirname, '..', 'gsd-core', 'workflows', 'execute-phase.md');
 const EXECUTOR_AGENT = path.join(__dirname, '..', 'agents', 'gsd-executor.md');
@@ -70,6 +71,36 @@ describe('prompt thinning — sub-200K context window support (#1978)', () => {
         'gsd-planner.md must reference planner-antipatterns.md for extended checkpoint anti-patterns and specificity examples'
       );
     });
+
+    test('sequences known external review after internal review fixes (#4107)', () => {
+      const planner = fs.readFileSync(PLANNER_AGENT, 'utf-8');
+      const assignWaves = planner.match(/<step name="assign_waves">([\s\S]{0,3000}?)<\/step>/)?.[1] || '';
+      assert.match(assignWaves, /known automatic external review/i);
+      assert.match(assignWaves, /plan includes internal review lanes/i);
+      assert.match(assignWaves, /accepted internal-review fixes[^.]*before[^.]*open/i);
+      assert.match(assignWaves, /run internal review and apply the accepted internal-review fixes before the final open/i);
+      assert.match(assignWaves, /if an open-time property exists[^.]*re-check it immediately before opening[^.]*nothing intervening/i);
+      assert.match(assignWaves, /post-open CI[^.]*review[^.]*tracking may follow/i);
+      // Directionality fixture (#4107): an inverted-order sentence — PR opens
+      // first, fixes land after — must fail the ordering-sensitive assertion
+      // above. Proves the regex tests sequence, not mere keyword co-occurrence.
+      const invertedOrdering = 'Open the PR before scheduling the accepted internal-review fixes.';
+      assert.doesNotMatch(invertedOrdering, /accepted internal-review fixes[^.]*before[^.]*open/i);
+      const invertedPlannerText = 'Open the PR, then run internal review and apply the accepted internal-review fixes.';
+      assert.doesNotMatch(invertedPlannerText, /run internal review and apply the accepted internal-review fixes before the final open/i);
+      assert.match(assignWaves, /planner-antipatterns\.md \("External Review Before PR Open \(#4107\)"\)/);
+
+      const reference = fs.readFileSync(PLANNER_ANTIPATTERNS_REF, 'utf-8');
+      const example = collectSection(reference, (heading) => heading.text === 'External Review Before PR Open (#4107)')?.body || '';
+      assert.match(example, /Bad[\s\S]*open[^\n]*PR[\s\S]*internal review/i);
+      assert.match(example, /Good[\s\S]*internal review[\s\S]*accepted fixes[\s\S]*if applicable[^\n]*re-check[^\n]*then immediately open PR/i);
+      assert.match(example, /nothing may intervene[^.]*re-check[^.]*open/i);
+      assert.match(example, /post-open work may follow/i);
+      // Directionality fixture: a Good section that opens the PR before
+      // internal review must fail the "Good" assertion above.
+      const invertedExample = 'Good:\nWave 1: If applicable, re-check the open-time property; then immediately open PR\nWave 2: Run internal review\nWave 3: Apply accepted fixes\n';
+      assert.doesNotMatch(invertedExample, /Good[\s\S]*internal review[\s\S]*accepted fixes[\s\S]*if applicable[^\n]*re-check[^\n]*then immediately open PR/i);
+    });
   });
 
   describe('executor-examples.md — extracted reference file', () => {