gstack-diff-scope reports every scope false on uncommitted work, and can't set two categories for one file
Version: 1.60.1.0
File: bin/gstack-diff-scope
Two independent bugs. Both fail silently — SCOPE_* comes back false rather than erroring, so the consuming skill just skips a reviewer and prints nothing. I hit the first one during a /ship and only noticed because I knew the diff was frontend-only.
Bug 1 — uncommitted work is invisible, so /ship skips reviewers
git diff BASE...HEAD --name-only exits 0 with empty output when the branch has no commits yet. The || fallback is therefore never reached:
# bin/gstack-diff-scope:10
FILES=$(git diff "${BASE}...HEAD" --name-only 2>/dev/null || git diff "${BASE}" --name-only 2>/dev/null || echo "")
FILES is empty, the early-exit block at :12-23 prints all-false, and every scope-gated reviewer is skipped.
This is not an edge case for /ship. /ship detects scope in Step 9 (Pre-Landing Review), which runs before it commits in Step 15. So the common flow — start work, run /ship with changes still in the working tree — hits this every time.
Repro
git checkout -b feat/x # no commits yet
echo "// x" >> src/Button.jsx # uncommitted frontend change
gstack-diff-scope main
# SCOPE_FRONTEND=false ← expected true
# ...all false
git add -A && git commit -m x
gstack-diff-scope main
# SCOPE_FRONTEND=true ← same diff, different answer
Impact
In ship/sections/review-army.md, specialist dispatch is gated on these flags:
SCOPE_FRONTEND → design specialist
SCOPE_AUTH / SCOPE_BACKEND → security specialist
SCOPE_MIGRATIONS → data-migration specialist (tagged [NEVER_GATE], i.e. meant to run even when silent)
An uncommitted migration or auth change silently skips the specialist that exists specifically to catch it. Step 1 also uses SCOPE_FRONTEND to decide whether to recommend /design-review, so that recommendation never fires either.
Suggested fix
Union the committed diff with the working tree and untracked files:
COMMITTED=$(git diff "${BASE}...HEAD" --name-only 2>/dev/null || true)
WORKING=$(git diff HEAD --name-only 2>/dev/null || true)
UNTRACKED=$(git ls-files --others --exclude-standard 2>/dev/null || true)
FILES=$(printf '%s\n%s\n%s\n' "$COMMITTED" "$WORKING" "$UNTRACKED" | sed '/^$/d' | sort -u)
Untracked matters too: a brand-new .jsx component or test file is exactly the kind of thing a reviewer should see, and /ship commits it in Step 15 regardless.
Bug 2 — one case for nine independent flags makes categories mutually exclusive
Lines :35-83 classify every file through a single case. case is first-match-wins, so each file can only ever set one category, even though the nine SCOPE_* values are independent booleans.
*.jsx (:39) precedes *.test.* (:53), so:
gstack-diff-scope main # diff contains only src/Button.test.jsx
# SCOPE_FRONTEND=true
# SCOPE_TESTS=false ← it is unambiguously a test file
The asymmetry is visible within the same run: util.test.ts sets TESTS (because *.test.* at :53 precedes *.ts at :81) while Button.test.jsx sets FRONTEND and not TESTS. Same intent, opposite result, purely from arm ordering.
Suggested fix
One case per category so the flags are genuinely independent.
BACKEND is the one that must stay exclusive — the # Backend: everything else that's code (excluding views/components already matched) comment at :76 shows it currently relies on fall-through to avoid claiming .tsx/.jsx. Make that explicit with a per-file frontend flag:
f_frontend=false
case "$f" in
*.tsx|*.jsx|*.vue|*.svelte|*.astro) f_frontend=true ;;
# ...remaining frontend patterns
esac
[ "$f_frontend" = true ] && FRONTEND=true
# ...independent case blocks for PROMPTS / TESTS / DOCS / CONFIG /
# MIGRATIONS / API / AUTH
if [ "$f_frontend" = false ]; then
case "$f" in
*.rb|*.py|*.go|*.rs|*.java|*.php|*.ex|*.exs) BACKEND=true ;;
*.ts|*.js|*.mjs|*.cjs|*.mts|*.cts) BACKEND=true ;;
esac
fi
Known behaviour change
With independent flags, util.test.ts sets BACKEND and TESTS where it previously set only TESTS. Backend-test-only PRs can then trip the security specialist via the SCOPE_BACKEND=true AND DIFF_LINES>100 gate. That errs toward more review rather than less, but it's a real change and worth a deliberate call.
Verification
I patched both locally and checked 15 cases:
| Input |
Before |
After |
uncommitted .jsx, no commits |
(all false) |
FRONTEND |
untracked new .jsx |
(all false) |
FRONTEND |
Button.test.jsx |
FRONTEND |
FRONTEND TESTS |
Button.spec.tsx |
FRONTEND |
FRONTEND TESTS |
util.test.ts |
TESTS |
BACKEND TESTS |
A.jsx |
FRONTEND |
FRONTEND |
server.ts |
BACKEND |
BACKEND |
app.mjs |
BACKEND |
BACKEND |
db/migrate/001_x.rb |
BACKEND MIGRATIONS |
BACKEND MIGRATIONS |
src/api/users.ts |
BACKEND API |
BACKEND API |
src/lib/auth.ts |
BACKEND AUTH |
BACKEND AUTH |
README.md |
DOCS |
DOCS |
package.json |
CONFIG |
CONFIG |
| clean tree, no changes |
(all false) |
(all false) |
Happy to open a PR if useful.
Minor
*.module.css|*.module.scss at :38 are unreachable — they sit in the same arm as *.css|*.scss, which already match them.
gstack-diff-scopereports every scope false on uncommitted work, and can't set two categories for one fileVersion: 1.60.1.0
File:
bin/gstack-diff-scopeTwo independent bugs. Both fail silently —
SCOPE_*comes backfalserather than erroring, so the consuming skill just skips a reviewer and prints nothing. I hit the first one during a/shipand only noticed because I knew the diff was frontend-only.Bug 1 — uncommitted work is invisible, so
/shipskips reviewersgit diff BASE...HEAD --name-onlyexits 0 with empty output when the branch has no commits yet. The||fallback is therefore never reached:FILESis empty, the early-exit block at :12-23 prints all-false, and every scope-gated reviewer is skipped.This is not an edge case for
/ship./shipdetects scope in Step 9 (Pre-Landing Review), which runs before it commits in Step 15. So the common flow — start work, run/shipwith changes still in the working tree — hits this every time.Repro
Impact
In
ship/sections/review-army.md, specialist dispatch is gated on these flags:SCOPE_FRONTEND→ design specialistSCOPE_AUTH/SCOPE_BACKEND→ security specialistSCOPE_MIGRATIONS→ data-migration specialist (tagged[NEVER_GATE], i.e. meant to run even when silent)An uncommitted migration or auth change silently skips the specialist that exists specifically to catch it. Step 1 also uses
SCOPE_FRONTENDto decide whether to recommend/design-review, so that recommendation never fires either.Suggested fix
Union the committed diff with the working tree and untracked files:
Untracked matters too: a brand-new
.jsxcomponent or test file is exactly the kind of thing a reviewer should see, and/shipcommits it in Step 15 regardless.Bug 2 — one
casefor nine independent flags makes categories mutually exclusiveLines :35-83 classify every file through a single
case.caseis first-match-wins, so each file can only ever set one category, even though the nineSCOPE_*values are independent booleans.*.jsx(:39) precedes*.test.*(:53), so:The asymmetry is visible within the same run:
util.test.tssetsTESTS(because*.test.*at :53 precedes*.tsat :81) whileButton.test.jsxsetsFRONTENDand notTESTS. Same intent, opposite result, purely from arm ordering.Suggested fix
One
caseper category so the flags are genuinely independent.BACKENDis the one that must stay exclusive — the# Backend: everything else that's code (excluding views/components already matched)comment at :76 shows it currently relies on fall-through to avoid claiming.tsx/.jsx. Make that explicit with a per-file frontend flag:Known behaviour change
With independent flags,
util.test.tssetsBACKENDandTESTSwhere it previously set onlyTESTS. Backend-test-only PRs can then trip the security specialist via theSCOPE_BACKEND=true AND DIFF_LINES>100gate. That errs toward more review rather than less, but it's a real change and worth a deliberate call.Verification
I patched both locally and checked 15 cases:
.jsx, no commitsFRONTEND.jsxFRONTENDButton.test.jsxFRONTENDFRONTEND TESTSButton.spec.tsxFRONTENDFRONTEND TESTSutil.test.tsTESTSBACKEND TESTSA.jsxFRONTENDFRONTENDserver.tsBACKENDBACKENDapp.mjsBACKENDBACKENDdb/migrate/001_x.rbBACKEND MIGRATIONSBACKEND MIGRATIONSsrc/api/users.tsBACKEND APIBACKEND APIsrc/lib/auth.tsBACKEND AUTHBACKEND AUTHREADME.mdDOCSDOCSpackage.jsonCONFIGCONFIGHappy to open a PR if useful.
Minor
*.module.css|*.module.scssat :38 are unreachable — they sit in the same arm as*.css|*.scss, which already match them.