mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 22:48:57 +02:00
Garry TanandClaude Fable 5
73cf0ed69a
Three silent-skip classes in bin/gstack-diff-scope, each of which quietly disabled scope-gated reviewers in /ship and /review: 1. Pattern gaps (#2526, #2455). `*/api/*` required a path segment BEFORE api/, so a root-level api/ layout (Vercel serverless, Next.js pages/api at root) never set SCOPE_API — 63 serverless functions in the reporter's payments repo, none ever classified, the API-contract specialist silently skipped on every payment PR (it found a CRITICAL when run by hand). Same for root-level migrations/. And the Rails data_migrate gem's db/data/ data migrations — arbitrary Ruby run unattended against production data — fell through to plain BACKEND, so the [NEVER_GATE] data-migration specialist never got the chance to run. Added: api/*, migrations/*, db/data/*, data_migrations/*. 2. All-false was indistinguishable from "could not look" (#2526). New contract: empty change set → all false exit 0; >=1 match → flags exit 0; changed files with ZERO matches → SCOPE_ERROR=unmatched + the unmatched paths as comment lines + exit 2 (a new top-level layout now trips loudly instead of invisibly disabling reviewers); unresolvable base ref (shallow CI checkout) → SCOPE_ERROR=no_base + exit 2 instead of a green that means "we could not look". Every output line stays a shell-safe assignment or comment for sourcing consumers, which tolerate the nonzero exit today (source ... || true / eval). 3. Uncommitted work was invisible (#2299). /ship detects scope in Step 9, BEFORE it commits in Step 15, so the common start-work-then-ship flow ran the classifier against an empty diff and skipped every reviewer. The change set is now the UNION of committed diff + working tree + untracked files. Also from #2299: the single first-match-wins case made the nine flags mutually exclusive (Button.test.jsx set FRONTEND but not TESTS; util.test.ts the opposite) — each category now gets its own case, with BACKEND deliberately still excluding frontend component/view files. And file listing is NUL-safe (git diff -z), so non-ASCII paths no longer defeat extension globs via octal quoting. Deliberate behavior change (flagged in #2299): with independent flags, a backend test file sets BACKEND and TESTS, which can trip the security specialist's SCOPE_BACKEND gate on test-only PRs — errs toward more review, not less. Table-driven tests cover every glob class (root api/, nested api/, controllers, openapi, root/nested/prisma/db-migrate/db-data migrations, dual-category test files, auth, prompts, docs, plain classes), the four-state exit contract, dirty-tree + untracked visibility, and the non-ASCII path case (39 pass in test/diff-scope.test.ts). Fixes shaped by the reporters' patches: @grant-ship-it (#2526), @mkyed (#2455), @ShahriarLak (#2299). Fixes #2526 Fixes #2455 Fixes #2299 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>