mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-02 23:14:33 +01:00
v1.93.16.0 fix(review): Step 11.5 bind ignores untracked root dot-dir scratch (GST-985) (#3873)
* fix(review): Step 11.5 bind ignores untracked root dot-dir scratch, keeps .github/ and .gstack/ (GST-985) gstack-review-log and gstack-review-read fingerprint with gstack-wtree --review, which leaves out root dot-directories that hold no tracked file. A test lane's .review-units-test-* scratch appearing or vanishing mid-review no longer unbinds the Step 9 row; .github/ and .gstack/ always count, and every other gstack-wtree caller keeps the whole tree. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes (Step 11: read-tree fallback and ..x root dirs) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: GST-985 changelog fragment and troubleshooting note Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(review): keep review-army.md at its no-loss-pinned bytes (GST-985) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
90ccf30e2a
commit
9cc40e0d03
9 files changed
+120
-7
No files matched your search
@@ -1,5 +1,11 @@
|
||||
# Changelog
|
||||
|
||||
## [1.93.16.0] - 2026-11-02
|
||||
|
||||
### Fixed
|
||||
|
||||
- **A test lane's scratch folder no longer breaks the /ship review bind.** When an untracked `.review-units-test-*/` folder appeared or vanished while reviewers ran, Step 11.5 saw a different tree and the only repair was rerunning Steps 9 to 11.5 (about 20M tokens). Review records now fingerprint with `gstack-wtree --review`, which leaves out root dot-folders that hold no tracked file. `.github/` and `.gstack/` always count, and any change to a tracked file still breaks the bind. The pregate, test evidence and every other caller keep the whole tree. (GST-985)
|
||||
|
||||
## [1.93.15.0] - 2026-11-02
|
||||
|
||||
**Pregate stops blaming a clean branch for a missing pytest, Playwright specs and mixed-language root tests, and the literals check stops flagging stray digits (#3598, #3492, gstack-evals#97).** A machine without pytest reported "fix the in-branch failure". Playwright specs in a vitest repo went to vitest, which found no tests and failed. Root tests in `.mjs` and `.py` got no runner at all. And any lone `2` in a file that named a changed constant (a comment, a `$2` in a shell stub, a fixture) failed `literals`. A wrapper bump from 3 to 4 would have raised 5 false hits here, and gstack-evals#97 hit 11. Rewritten from outside PRs #3639 and #3600 by @vilano93 and #3629 by @nfitzsimmons.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
# gstack digest v1.93.15.0 — regenerate/re-copy after upgrading gstack
|
||||
# gstack digest v1.93.16.0 — regenerate/re-copy after upgrading gstack
|
||||
|
||||
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
|
||||
for agent hosts without a full skill install. The full skills add workflows,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
+28
-1
@@ -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,21 @@ 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
|
||||
TRACKED_INDEX="$TMPIDX"
|
||||
if [ -n "$REAL_INDEX" ] && [ -f "$REAL_INDEX" ]; then TRACKED_INDEX="$REAL_INDEX"; fi
|
||||
for DOTDIR in "$TOP"/.[!.]* "$TOP"/..?*; do
|
||||
NAME=${DOTDIR##*/}
|
||||
case "$NAME" in .git|.github|.gstack|.bun) continue ;; esac
|
||||
if [ ! -d "$DOTDIR" ] || [ -L "$DOTDIR" ]; then continue; fi
|
||||
if GIT_INDEX_FILE="$TRACKED_INDEX" 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
|
||||
|
||||
@@ -570,6 +570,9 @@ the base has moved.
|
||||
**Meaning.** The working tree changed between the pass's `gstack-review-log --start` and
|
||||
its logged row: an edit, commit or merge landed while reviewers were still reading. The
|
||||
reviewers may not have seen that change, so the row's `review_binding.state` is `changed`.
|
||||
Review records fingerprint with `gstack-wtree --review`, which skips untracked root dot-folders
|
||||
that hold no tracked file (never `.github/` or `.gstack/`), so a test lane's scratch folder alone
|
||||
does not move the tree.
|
||||
Step 11.5 cannot bind it and inserts `9 → 10 → 11 → 11.5`. Logging the same results again
|
||||
cannot repair it.
|
||||
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gstack",
|
||||
"version": "1.93.15",
|
||||
"version": "1.93.16",
|
||||
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
|
||||
"license": "MIT",
|
||||
"type": "module",
|
||||
|
||||
@@ -166,6 +166,83 @@ 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'));
|
||||
});
|
||||
|
||||
test('GST-985: an untracked file in a dot-directory that holds a tracked file still breaks the bind', () => {
|
||||
mkdirSync(join(repo, '.tool'));
|
||||
writeFileSync(join(repo, '.tool', 'a.json'), '{}\n');
|
||||
git('add', '.tool/a.json');
|
||||
git('commit', '-qm', 'tool');
|
||||
const token = cli('gstack-review-log', ['--start', 'review']);
|
||||
writeFileSync(join(repo, '.tool', 'b.json'), '{}\n');
|
||||
expect(log(token).review_binding.state).toBe('changed');
|
||||
});
|
||||
|
||||
test('GST-985: a root ..x scratch dir created mid-review still binds', () => {
|
||||
const token = cli('gstack-review-log', ['--start', 'review']);
|
||||
mkdirSync(join(repo, '..x'));
|
||||
writeFileSync(join(repo, '..x', 'unit.json'), '{}\n');
|
||||
expect(log(token).review_binding.state).toBe('verified');
|
||||
});
|
||||
|
||||
test('GST-985: a git add -N file in a new dot-directory counts under the read-tree HEAD fallback', () => {
|
||||
mkdirSync(join(repo, '.newdir'));
|
||||
writeFileSync(join(repo, '.newdir', 'a.ts'), 'export const a = 1;\n');
|
||||
git('add', '-N', '.newdir/a.ts');
|
||||
const fakeBin = mkdtempSync(join(tmpdir(), 'review-start-faketouch-'));
|
||||
writeFileSync(join(fakeBin, 'touch'), '#!/bin/sh\nexit 1\n');
|
||||
chmodSync(join(fakeBin, 'touch'), 0o755);
|
||||
try {
|
||||
const env = { PATH: `${fakeBin}:${process.env.PATH}` };
|
||||
expect(cli('gstack-wtree', ['--review'], repo, env)).toBe(cli('gstack-wtree', [], repo, env));
|
||||
} finally {
|
||||
rmSync(fakeBin, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('GST-985: gstack-wtree refuses an unknown or extra argument', () => {
|
||||
for (const args of [['--bogus'], ['--review', 'x']]) {
|
||||
const r = spawnSync(join(ROOT, 'bin', 'gstack-wtree'), args, { cwd: repo, encoding: 'utf8', timeout: 10_000 });
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain('Usage: gstack-wtree [--review]');
|
||||
}
|
||||
});
|
||||
|
||||
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');
|
||||
|
||||
Reference in new issue
Block a user