diff --git a/bin/gstack-review-log b/bin/gstack-review-log index 1fabcc3b2..b469ee1a4 100755 --- a/bin/gstack-review-log +++ b/bin/gstack-review-log @@ -69,7 +69,7 @@ if [ -n "$COMMIT_FULL" ]; then esac trap 'rm -rf "$WTREE_OBJECTS"' EXIT trap 'exit 130' INT TERM - WTREE=$(GSTACK_WTREE_OBJECT_DIR="$WTREE_OBJECTS" "$SCRIPT_DIR/gstack-wtree" || true) + WTREE=$(GSTACK_WTREE_OBJECT_DIR="$WTREE_OBJECTS" "$SCRIPT_DIR/gstack-wtree" --review || true) export GIT_ALTERNATE_OBJECT_DIRECTORIES="$WTREE_OBJECTS${GIT_ALTERNATE_OBJECT_DIRECTORIES:+:$GIT_ALTERNATE_OBJECT_DIRECTORIES}" else echo "gstack-review-log: cannot create a private object directory under ${TMPDIR:-/tmp}; no working-tree fingerprint. Set TMPDIR to a writable directory." >&2 diff --git a/bin/gstack-review-read b/bin/gstack-review-read index e673640ab..9b1530c62 100755 --- a/bin/gstack-review-read +++ b/bin/gstack-review-read @@ -4,7 +4,7 @@ # # Emits, in order: reviews JSONL with computed review_freshness, ---CONFIG--- (skip_eng_review), # ---HEAD--- (short sha), ---WTREE--- (current working-tree fingerprint from -# bin/gstack-wtree, or "unknown"), ---TREE--- (HEAD tree, informational) and +# bin/gstack-wtree --review, or "unknown"), ---TREE--- (HEAD tree, informational) and # ---DIRTY--- (tracked-file dirty flag). Consumers grade diff-scoped review # rows using review_freshness; legacy or uncaptured diff evidence cannot fall # back to HEAD equality. Plan rows are unchanged. @@ -19,7 +19,7 @@ SLUG=$("$SCRIPT_DIR/gstack-slug" --get REPO_SLUG 2>/dev/null || true) . "$(dirname "$0")/gstack-state-root.sh" 2>/dev/null || { echo "$0: cannot resolve the gstack state root: $(dirname "$0")/gstack-state-root.sh is missing. fix: reinstall with ./setup or /gstack-upgrade (docs/state-root.md)" >&2; exit 1; } gstack_state_root_select GSTACK_HOME="$_gstack_sr_root" -WTREE=$("$SCRIPT_DIR/gstack-wtree" 2>/dev/null || echo "unknown") +WTREE=$("$SCRIPT_DIR/gstack-wtree" --review 2>/dev/null || echo "unknown") if [ -f "$GSTACK_HOME/projects/$SLUG/$BRANCH-reviews.jsonl" ]; then GSTACK_REVIEW_LIB="$SCRIPT_DIR/../lib/review-evidence.ts" GSTACK_REVIEW_WTREE="$WTREE" bun --no-env-file --config="${GSTACK_BUN_CONFIG:-/dev/null}" -e ' const { reviewFreshness } = await import(process.env.GSTACK_REVIEW_LIB); diff --git a/bin/gstack-wtree b/bin/gstack-wtree index c8034e375..87e3ae1b3 100755 --- a/bin/gstack-wtree +++ b/bin/gstack-wtree @@ -9,6 +9,12 @@ # look like a mid-run edit (GST-421); tracked files under it still count, and a # tree without that folder (or with it gitignored) hashes exactly as a plain # `git add -A` would. +# +# --review (gstack-review-log and gstack-review-read only, GST-985) also leaves +# out every root dot-directory that holds no tracked file, such as a test +# lane's `.review-units-test-*/` scratch, so scratch appearing or vanishing +# mid-review cannot unbind a review. `.github/` and `.gstack/` always stay in: +# they hold CI and gate policy. Every other caller keeps the whole tree. # Properties that make this the right staleness fingerprint, vs # `git rev-parse HEAD^{tree}`: # @@ -40,6 +46,13 @@ # treat that as "no fingerprint"; a partial hash is never printed. set -euo pipefail +REVIEW="" +case "$#:${1:-}" in + 0:) ;; + 1:--review) REVIEW=1 ;; + *) echo "Usage: gstack-wtree [--review]" >&2; exit 2 ;; +esac + TOP=$(git rev-parse --show-toplevel 2>/dev/null) || exit 1 # Resolve the REAL index path BEFORE exporting GIT_INDEX_FILE — with the env # var set, `git rev-parse --git-path index` returns the temp index itself and @@ -95,7 +108,19 @@ fi # call .bun not ignored. EXCLUDE_BUN="" if [ -e "$TOP/.bun" ] && ! git -C "$TOP" check-ignore --no-index -q .bun 2>/dev/null; then EXCLUDE_BUN=':(exclude,top).bun'; fi -if ! ERR=$(git -C "$TOP" add -u 2>&1 >/dev/null) || ! ERR=$(git -C "$TOP" add -A -- . ${EXCLUDE_BUN:+"$EXCLUDE_BUN"} 2>&1 >/dev/null); then +EXCLUDES=() +if [ -n "$EXCLUDE_BUN" ]; then EXCLUDES+=("$EXCLUDE_BUN"); fi +if [ -n "$REVIEW" ]; then + for DOTDIR in "$TOP"/.[!.]*; do + NAME=${DOTDIR##*/} + case "$NAME" in .git|.github|.gstack|.bun) continue ;; esac + if [ ! -d "$DOTDIR" ] || [ -L "$DOTDIR" ]; then continue; fi + if git -C "$TOP" ls-files --error-unmatch -- ":(top,literal)$NAME" >/dev/null 2>&1; then continue; fi + if git -C "$TOP" check-ignore --no-index -q -- "$NAME" 2>/dev/null; then continue; fi + EXCLUDES+=(":(exclude,top,literal)$NAME") + done +fi +if ! ERR=$(git -C "$TOP" add -u 2>&1 >/dev/null) || ! ERR=$(git -C "$TOP" add -A -- . ${EXCLUDES[@]+"${EXCLUDES[@]}"} 2>&1 >/dev/null); then echo "gstack-wtree: cannot stage the working tree: $(printf '%s' "$ERR" | head -1)" >&2 exit 1 fi diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 450f3adae..f3ac2ef4a 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -102,7 +102,7 @@ This pass is static; defer product probes to Step 9.2.1. 1. Read `~/.claude/skills/gstack/review/checklist.md`. If the file cannot be read, **STOP** and report the error. -2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, first reserve this pass's dispatch batch with `gstack-ship-budget check` and add `--reservation `). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. +2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, first reserve this pass's dispatch batch with `gstack-ship-budget check` and add `--reservation `). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them, except a root dot-directory with no tracked file (test scratch), which it skips so scratch cannot unbind the review; `.github/` and `.gstack/` (CI and gate policy) always count. Stage new source there with `git add -N`. Every pass captures a full token (no `--fix-range`), so the record binds the whole tree even when reviewers read only the range. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 8b349c4e9..bb1c1590c 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -47,7 +47,7 @@ This pass is static; defer product probes to Step 9.2.1. 1. Read `~/.claude/skills/gstack/review/checklist.md`. If the file cannot be read, **STOP** and report the error. -2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, first reserve this pass's dispatch batch with `gstack-ship-budget check` and add `--reservation `). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them. +2. Before reading the diff, run `~/.claude/skills/gstack/bin/gstack-review-log --start review` and save its token as REVIEW_START (under a token cap, first reserve this pass's dispatch batch with `gstack-ship-budget check` and add `--reservation `). Then run `git diff origin/` (`git diff $RANGE_BASE` from pass 2 on). Read non-ignored untracked source files too (`git ls-files --others --exclude-standard`); the snapshot includes them, except a root dot-directory with no tracked file (test scratch), which it skips so scratch cannot unbind the review; `.github/` and `.gstack/` (CI and gate policy) always count. Stage new source there with `git add -N`. Every pass captures a full token (no `--fix-range`), so the record binds the whole tree even when reviewers read only the range. diff --git a/test/review-start-evidence.test.ts b/test/review-start-evidence.test.ts index eae485fd4..79bfa6785 100644 --- a/test/review-start-evidence.test.ts +++ b/test/review-start-evidence.test.ts @@ -166,6 +166,43 @@ describe('review start/end binding (#2803)', () => { }); } + for (const when of ['created', 'removed']) { + test(`GST-985: an untracked scratch dir ${when} mid-review still binds; a tracked edit still breaks it`, () => { + const scratchDir = join(repo, '.review-units-test-x'); + if (when === 'removed') { mkdirSync(scratchDir); writeFileSync(join(scratchDir, 'unit.json'), '{}\n'); } + const token = cli('gstack-review-log', ['--start', 'review']); + if (when === 'created') { mkdirSync(scratchDir); writeFileSync(join(scratchDir, 'unit.json'), '{}\n'); } + else rmSync(scratchDir, { recursive: true, force: true }); + const row = log(token); + expect(row.review_binding.state).toBe('verified'); + expect(row.review_binding.start_wtree).toBe(row.review_binding.end_wtree); + expect(row.review_freshness.status).toBe('CURRENT'); + expect(git('diff', '--name-only', row.wtree, cli('gstack-wtree'))).toBe(when === 'created' ? '.review-units-test-x/unit.json' : ''); + + const edited = cli('gstack-review-log', ['--start', 'review']); + writeFileSync(join(repo, 'source.ts'), 'export const unreviewed = true;\n'); + expect(log(edited).review_binding.state).toBe('changed'); + }); + } + + test('GST-985: ignored root dot-directories still fingerprint under --review', () => { + writeFileSync(join(repo, '.git', 'info', 'exclude'), '.cache/\n'); + mkdirSync(join(repo, '.cache')); + writeFileSync(join(repo, '.cache', 'blob'), 'x\n'); + expect(cli('gstack-wtree', ['--review'])).toBe(cli('gstack-wtree')); + }); + + for (const file of ['.github/workflows/x.yml', '.gstack/ship-policy.json']) { + test(`GST-985: an untracked ${file} added mid-review still breaks the bind`, () => { + const token = cli('gstack-review-log', ['--start', 'review']); + mkdirSync(join(repo, file, '..'), { recursive: true }); + writeFileSync(join(repo, file), '{}\n'); + const row = log(token); + expect(row.review_binding.state).toBe('changed'); + expect(git('diff', '--name-only', row.review_binding.start_wtree, row.review_binding.end_wtree)).toBe(file); + }); + } + test('fix commits do not certify the final tree until a new unchanged pass', () => { const token = cli('gstack-review-log', ['--start', 'review']); writeFileSync(join(repo, 'source.ts'), 'export const fixed = true;\n'); diff --git a/test/size-ledger/ship/gst-985-review-bind-scratch.json b/test/size-ledger/ship/gst-985-review-bind-scratch.json new file mode 100644 index 000000000..9e20da1c6 --- /dev/null +++ b/test/size-ledger/ship/gst-985-review-bind-scratch.json @@ -0,0 +1,4 @@ +{ + "bytes": 220, + "why": "GST-985: review-army says the review snapshot skips untracked root dot-dir scratch but keeps .github/ and .gstack/" +}