fix(#4466): bound quick.md's post-execute review scope tip at the task's own last commit (#4571)

* fix(#4466): bound quick.md's post-execute review scope tip at the task's own last commit

The review-scoping step computed CHANGED_FILES as `git diff --name-only
"${DIFF_BASE}..HEAD"`. DIFF_BASE is correctly bound to the quick task's
start (via the oldest QUICK_COMMITS entry's parent), but the tip was bare
HEAD -- unbounded. Anything landing on the shared tree between the task's
own commits and this review step running (a worktree merge-back, another
session sharing the tree) got folded into the quick task's own review
scope.

QUICK_COMMITS (newest-first) already holds the correct tip as its first
line -- read QUICK_TIP from the value already computed, diff against
that instead of HEAD. No new derivation, no new git call.

Added tests/quick-review-scope-tip-bound.test.cjs: extracts the scoping
fence verbatim from quick.md and runs it against a real git fixture
matching the issue's own scenario (quick task's own commit, then a later
unrelated commit on the shared tree). Manually verified watch-it-fail
(bare HEAD includes the unrelated file) / watch-it-pass (bounded tip
excludes it) via direct bash execution before wiring the test file,
since this repo blocks local node --test.

Independent code review caught one drive-by finding: an allow-test-rule
marker copied from a sibling test's pattern was unnecessary here (and
there) -- local/no-source-grep's looksLikeSourcePath only matches
readFileSync targets ending in .cjs/.cts/.js/.mjs/.mts/.ts, never .md,
so the rule can never fire regardless of the marker. Confirmed by
reading eslint-rules/no-source-grep.cjs directly; removed.

Emitted-Drift-Ack-Growth: quick.md — the fix adds a QUICK_TIP line and its explanatory comment; not a regeneration artifact, a hand-authored bug fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* docs(#4466): backfill changeset PR number

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

---------

Co-authored-by: sim <sim@local>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

.changeset/vivid-quails-roam.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 4571
+---
+**`/gsd-quick`'s post-execute review no longer scopes past its own last commit** — the review-scoping step diffed against bare HEAD instead of the quick task's own newest commit, so any later commit landing on the same tree before the review ran (a worktree merge-back, a shared tree) was silently folded into the quick task's own code-review scope.

gsd-core/workflows/quick.md

@@ -569,7 +569,14 @@ else
 fi
 
 if [ -n "$DIFF_BASE" ]; then
-  CHANGED_FILES=$(git diff --name-only "${DIFF_BASE}..HEAD" -- . ':!.planning' 2>/dev/null | tr '\n' ' ')
+  # #4466: bound the tip at the quick task's own last commit, not HEAD --
+  # QUICK_COMMITS is already the complete, newest-first list of this task's
+  # commits, so its first line is the correct tip. An unbounded `..HEAD`
+  # picks up any later commit landed on the same tree in the window between
+  # this task's commits and this review step (worktree merge-back, a shared
+  # tree, another session) and folds it into this task's own review scope.
+  QUICK_TIP=$(echo "$QUICK_COMMITS" | head -1)
+  CHANGED_FILES=$(git diff --name-only "${DIFF_BASE}..${QUICK_TIP}" -- . ':!.planning' 2>/dev/null | tr '\n' ' ')
 else
   CHANGED_FILES=""
 fi

tests/quick-review-scope-tip-bound.test.cjs

@@ -0,0 +1,130 @@
+'use strict';
+
+/**
+ * Regression coverage for #4466: quick.md's post-execute review scoping step
+ * computes `CHANGED_FILES` as `git diff --name-only "${DIFF_BASE}..HEAD"`.
+ * `DIFF_BASE` (the parent of QUICK_COMMITS's oldest entry) is correctly
+ * bound to the quick task's start, but the tip is bare HEAD — unbounded.
+ * Anything landing on the shared tree between the task's own commits and
+ * this review step running (a worktree merge-back, another session sharing
+ * the tree) gets folded into the quick task's own review scope.
+ *
+ * QUICK_COMMITS (newest-first) already holds the correct tip as its first
+ * line — the fix reads that instead of using HEAD, no new git call needed.
+ *
+ * Mirrors the issue's own verified reproduction methodology: extract the
+ * fence VERBATIM from quick.md (never reimplemented), run it against a real
+ * constructed git fixture matching the issue's own exact scenario (an
+ * unrelated commit landing after the quick task's own work, before review).
+ */
+
+const { describe, test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+const { spawnSync } = require('node:child_process');
+const { gitOrThrow } = require('./helpers/git-fixture.cjs');
+const { GIT_FIXTURE_TIMEOUT_MS } = require('./helpers/timeouts.cjs');
+const { createTempDir, cleanup } = require('./helpers.cjs');
+
+const WORKFLOW_PATH = path.join(__dirname, '..', 'gsd-core', 'workflows', 'quick.md');
+
+function extractFirstBashBlockAfter(content, startAnchor, stopAnchor) {
+  const start = content.indexOf(startAnchor);
+  assert.ok(start !== -1, `quick.md must contain the anchor "${startAnchor}"`);
+  const stop = stopAnchor ? content.indexOf(stopAnchor, start + startAnchor.length) : content.length;
+  assert.ok(!stopAnchor || stop !== -1, `quick.md must contain the anchor "${stopAnchor}" after "${startAnchor}"`);
+  const region = content.slice(start, stop);
+
+  const fenceStart = region.indexOf('```bash');
+  assert.ok(fenceStart !== -1, `no \`\`\`bash fence found between "${startAnchor}" and its stop anchor`);
+  const fenceEnd = region.indexOf('```', fenceStart + '```bash'.length);
+  assert.ok(fenceEnd !== -1, `unterminated \`\`\`bash fence after "${startAnchor}"`);
+  return region.slice(fenceStart + '```bash'.length, fenceEnd);
+}
+
+function seedFixtureRepo(dir) {
+  gitOrThrow(['init', '-q'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS });
+  gitOrThrow(['config', 'user.email', 't@example.com'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS });
+  gitOrThrow(['config', 'user.name', 'T'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS });
+  gitOrThrow(['config', 'commit.gpgsign', 'false'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS });
+}
+
+function writeAndCommit(dir, relPath, content, message) {
+  const abs = path.join(dir, relPath);
+  fs.mkdirSync(path.dirname(abs), { recursive: true });
+  fs.writeFileSync(abs, content);
+  gitOrThrow(['add', '-A'], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS });
+  gitOrThrow(['commit', '-q', '-m', message], { cwd: dir, timeoutMs: GIT_FIXTURE_TIMEOUT_MS });
+}
+
+// Matches the issue's own fixture exactly: init, then the quick task's own
+// commit (referencing quick_id in its message), then an unrelated later
+// commit landing on the same tree before the review step runs.
+const QUICK_ID = '260906-abc';
+
+function buildFixture(tmpDir) {
+  seedFixtureRepo(tmpDir);
+  writeAndCommit(tmpDir, 'README.md', '# init\n', 'chore: init');
+  writeAndCommit(tmpDir, 'src/quick-a.js', 'quick-a\n', `feat(quick-${QUICK_ID}): the quick task's own work`);
+  writeAndCommit(tmpDir, 'src/unrelated.js', 'unrelated\n', 'fix: an unrelated commit from another session on the shared tree');
+}
+
+function runScopingFence(tmpDir) {
+  const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
+  const fence = extractFirstBashBlockAfter(content, "**Scope files from executor's commits:**", '**Invoke review:**');
+
+  const script = [
+    '#!/usr/bin/env bash',
+    'set -uo pipefail',
+    `quick_id="${QUICK_ID}"`,
+    '{',
+    fence,
+    '} 1>&2',
+    'printf \'%s\\n\' "$CHANGED_FILES"',
+  ].join('\n');
+
+  const scriptPath = path.join(tmpDir, '.scope-script.sh');
+  fs.writeFileSync(scriptPath, script);
+
+  const result = spawnSync('bash', [scriptPath], {
+    cwd: tmpDir,
+    encoding: 'utf8',
+    timeout: GIT_FIXTURE_TIMEOUT_MS,
+  });
+  if (result.error) {
+    throw new Error(`bash spawn failed: ${result.error.message}\ndiagnostics:\n${result.stderr || '(none)'}`);
+  }
+  if (result.status !== 0) {
+    throw new Error(`bash exited ${result.status} (signal ${result.signal})\ndiagnostics:\n${result.stderr || '(none)'}`);
+  }
+  const files = result.stdout.trim().split(/\s+/).filter(Boolean).sort();
+  return { files, diagnostics: result.stderr };
+}
+
+describe('#4466: quick.md review scoping bounds the tip at the quick task\'s own last commit', () => {
+  const workflowContent = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
+  const fence = extractFirstBashBlockAfter(workflowContent, "**Scope files from executor's commits:**", '**Invoke review:**');
+
+  test('the diff uses a bounded tip, not bare HEAD (the gate exists)', () => {
+    assert.ok(
+      /git diff --name-only "\$\{DIFF_BASE\}\.\.\$\{QUICK_TIP\}"/.test(fence),
+      'the scoping fence must diff against a bounded QUICK_TIP, not bare HEAD',
+    );
+  });
+
+  test('real execution: a later unrelated commit on the shared tree is excluded from scope (issue #4466 repro)', () => {
+    const tmpDir = fs.realpathSync.native(createTempDir('gsd-4466-'));
+    try {
+      buildFixture(tmpDir);
+      const { files, diagnostics } = runScopingFence(tmpDir);
+      assert.deepEqual(
+        files,
+        ['src/quick-a.js'],
+        `quick task's review scope must not include a later unrelated commit, got: ${JSON.stringify(files)}\ndiagnostics:\n${diagnostics || '(none)'}`,
+      );
+    } finally {
+      cleanup(tmpDir);
+    }
+  });
+});