fix(#4460): rework test to not depend on Tier 2's broken bash (#4461)

A fresh code-review pass found the test's original approach (concatenate
and execute Tier 1 + Tier 2 + Tier 3 verbatim, matching the issue's own
reproduction) cannot run: Tier 2's own fence -- untouched by this diff --
is not currently parseable bash. Two unescaped `"` inside its embedded
`node -e "..."` regex literal (`raw.replace(/^['"]|['"]$/g, '')`)
terminate the outer double-quoted string early, which breaks bash's
PARSE of the whole concatenated script even though Tier 2's body never
executes under --files. Independently confirmed via manual extraction
and execution before accepting the finding.

This is a real, separately-filed, already-queued sibling issue (#4461,
filed by #4460's own reporter specifically to avoid folding it in here)
-- not fixed in this PR. Instead reworked the test to run only Tier 1 +
Tier 3 verbatim, seeding the Tier-2-equivalent REVIEW_FILES state
directly for the "without --files" case (documented in the module
docblock, explaining why Tier 2 isn't sourced and pointing at #4461).

Also fixed a nit from the same review pass: a code comment overstated
Tier 2's guard as "immediately above" when it's ~150 lines away.

Manually re-verified both test cases against a real git fixture with a
GNU-realpath-compatible `realpath` (matching gsd-test's Linux bench --
this Mac's BSD realpath lacks the `-m` flag Tier 1 uses, a SEPARATE
pre-existing portability gap surfaced during this check, masked on Linux
CI, not touched by this fix) before re-running the full suite.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

gsd-core/workflows/code-review.md

@@ -298,7 +298,7 @@ elif [ -z "$FILES_OVERRIDE" ] && [ -n "$DIFF_BASE" ]; then
   # reached anyway and the #2666 cross-check below appended the whole phase
   # diff onto an explicit user-supplied file list, contradicting line 144's
   # "Skip SUMMARY/git scoping entirely when --files is provided" and Tier 2's
-  # own --files guard immediately above.
+  # own --files guard (line 150).
   # #2666 cross-check: SUMMARY yielded a non-empty (possibly partial) scope.
   # Warn about — and add — any changed files the SUMMARY extractor did not surface,
   # so a partial result can no longer silently ship an incomplete review scope.

tests/code-review-tier3-files-override-scoping.test.cjs

@@ -14,9 +14,22 @@
  * whole phase diff onto an explicit user-supplied file list.
  *
  * Mirrors the issue's own verified reproduction methodology: extract the
- * Tier 1/2/3 fences VERBATIM from code-review.md (never reimplemented),
- * set only the prerequisite variables, run against a real constructed git
- * fixture.
+ * Tier 1 and Tier 3 fences VERBATIM from code-review.md (never
+ * reimplemented), set only the prerequisite variables, run against a real
+ * constructed git fixture.
+ *
+ * Tier 2's own fence is DELIBERATELY NOT extracted-and-executed here — a
+ * code-review pass on this fix found it is not currently parseable bash at
+ * all (two unescaped `"` characters inside its embedded `node -e "..."`
+ * regex literal terminate the outer double-quoted string early, breaking
+ * bash's parse of the WHOLE script even though Tier 2's body never
+ * executes under `--files`). That defect is real, already reported, and
+ * already queued as its own issue (#4461, filed separately by #4460's own
+ * reporter: "the Tier-2 SUMMARY-extraction fence is not parseable bash
+ * (same file, different defect)") — fixing it here would be exactly the
+ * scope creep the reporter took care to avoid. Until #4461 lands, the
+ * "without --files" case below seeds the REVIEW_FILES state Tier 2 would
+ * have produced directly, rather than sourcing Tier 2's broken fence.
  */
 
 const { describe, test } = require('node:test');
@@ -80,27 +93,38 @@ function buildFixture(tmpDir) {
   }
 }
 
-function runTiers(tmpDir, { filesOverride }) {
+/**
+ * Runs Tier 1 (verbatim) then Tier 3 (verbatim) in sequence. `seedReviewFiles`
+ * stands in for what Tier 2 would have produced when `filesOverride` is unset
+ * (Tier 2 itself is not sourced — see the module docblock for why) — an empty
+ * array when omitted, matching Tier 2's own real behavior when no SUMMARY
+ * yields anything.
+ */
+function runTiers(tmpDir, { filesOverride, seedReviewFiles = [] }) {
   const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
   const tier1 = extractFirstBashBlockAfter(content, '**Tier 1 — --files override', '**Tier 2 —');
-  const tier2 = extractFirstBashBlockAfter(content, '**Tier 2 — SUMMARY.md extraction', '**Tier 3 —');
   const tier3 = extractFirstBashBlockAfter(content, '**Tier 3 — Git diff fallback', '**Post-processing');
 
   const filesArrayInit = filesOverride
     ? `FILES_ARRAY=(${filesOverride})`
     : 'FILES_ARRAY=()';
+  const seedInit = seedReviewFiles.length
+    ? `REVIEW_FILES=(${seedReviewFiles.map((f) => `"${f}"`).join(' ')})`
+    : 'REVIEW_FILES=()';
 
   const script = [
     '#!/usr/bin/env bash',
     'set -uo pipefail',
     `FILES_OVERRIDE="${filesOverride || ''}"`,
     filesArrayInit,
-    'REVIEW_FILES=()',
+    tier1,
+    // Tier 1 unconditionally resets REVIEW_FILES=() when FILES_OVERRIDE is
+    // set; the seed only matters (and only applies) when it is not, exactly
+    // mirroring Tier 2 running in FILES_OVERRIDE's absence.
+    `if [ -z "$FILES_OVERRIDE" ]; then ${seedInit}; fi`,
     'PHASE_DIR=".planning/phases/03-demo"',
     'PADDED_PHASE="03"',
     'LAST_REVIEW_COMMIT=""',
-    tier1,
-    tier2,
     tier3,
     'printf \'%s\\n\' "${REVIEW_FILES[@]}"',
   ].join('\n');
@@ -145,15 +169,18 @@ describe('#4460: code-review.md Tier 3 does not widen an explicit --files overri
     }
   });
 
-  test('without --files, the #2666 cross-check still widens a partial SUMMARY scope (no regression to the cross-check itself)', () => {
+  test('without --files, the #2666 cross-check still widens a partial (Tier-2-equivalent) scope (no regression to the cross-check itself)', () => {
     const tmpDir = fs.realpathSync(createTempDir('gsd-4460-'));
     try {
       buildFixture(tmpDir);
-      const files = runTiers(tmpDir, { filesOverride: '' });
+      // seedReviewFiles stands in for Tier 2's real output (["src/alpha.js"],
+      // the file the fixture's SUMMARY lists) — see the module docblock for
+      // why Tier 2's own fence isn't sourced here.
+      const files = runTiers(tmpDir, { filesOverride: '', seedReviewFiles: ['src/alpha.js'] });
       assert.deepEqual(
         files,
         ['src/alpha.js', 'src/beta.js', 'src/delta.js', 'src/epsilon.js', 'src/gamma.js'],
-        `without --files, the cross-check must still widen the partial SUMMARY scope, got: ${JSON.stringify(files)}`,
+        `without --files, the cross-check must still widen a partial scope, got: ${JSON.stringify(files)}`,
       );
     } finally {
       cleanup(tmpDir);