diff --git a/CHANGELOG.md b/CHANGELOG.md index b0fb7b918..045d9f49a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [Unreleased] + +**`gstack-brain-sync --publish-removals --yes` no longer prints `list: unbound variable` when it finishes, and it now removes its temp list and releases the sync lock.** The command's EXIT trap named the function's local `$list`. When a publish succeeded, the function returned before the script exited, so the trap ran after `list` had gone out of scope. Under `set -u` it stopped at the unbound variable and never reached the cleanup, which left `brain-sync-removals.*` in `$TMPDIR` and `.brain-sync.lock.d` in `~/.gstack` until a later run cleared the lock as stale (found by GSTA-44). + +### Fixed +- **`bin/gstack-brain-sync`**: `--publish-removals` writes the temp file paths into its EXIT/INT/TERM trap when it installs the trap (quoted with `printf %q`), so the cleanup and lock release work however the function ends. The drain's trap in `--once` also names locals, but every path in that function ends with `exit` while they are still in scope, so it's unchanged. The other trap sites (the drain's lock-only trap and `gstack-brain-restore`'s, which uses a global) don't have the bug. + +### Tests +- `test/brain-sync-removals.test.ts`: a new case runs `--publish-removals`, then `--publish-removals --yes` twice (publishing, then nothing to publish) with a private `TMPDIR`. After each run, stderr has no `unbound variable`, the exit status is 0, `TMPDIR` is empty and the lock directory is gone. It failed before the fix. + ## [1.91.191.0] - 2026-10-25 **`gstack-version-bump write --changelog` moves the branch's `[Unreleased]` entry under the new version, and /ship Step 12 stops treating a provisional queue pick as a verified slot.** Stamping by hand meant writing VERSION with `gstack-version-bump write` and then renaming the `## [Unreleased]` heading yourself, because only `gstack-restamp` re-headed the CHANGELOG. Separately, when `gstack-next-version` could not read some open PRs' VERSION it returned `complete:false` and a provisional pick, and Step 12 printed the warning and then wrote that version as if the queue had been checked (left over from #3258, noted in GSTA-45). diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index 883e717e4..50f377b0b 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -1068,7 +1068,7 @@ subcmd_publish_removals() { sync_active || { echo "BRAIN_SYNC: artifacts sync is not active; nothing to publish" >&2; exit 1; } gstack_brain_lock_acquire "$GSTACK_HOME" try || { echo "BRAIN_SYNC: another sync is running; try again" >&2; exit 1; } list=$(mktemp "${TMPDIR:-/tmp}/brain-sync-removals.XXXXXX") || exit 1 - trap 'rm -f "$list" "$list.gone"; gstack_brain_lock_release' EXIT INT TERM + trap "rm -f $(printf '%q ' "$list" "$list.gone"); gstack_brain_lock_release" EXIT INT TERM removal_candidates "$list" || { echo "BRAIN_SYNC: could not list removed files" >&2; exit 1; } n=$(grep -c . "$list" 2>/dev/null || true) if [ "$n" -eq 0 ]; then diff --git a/test/brain-sync-removals.test.ts b/test/brain-sync-removals.test.ts index 66b457375..405de494d 100644 --- a/test/brain-sync-removals.test.ts +++ b/test/brain-sync-removals.test.ts @@ -16,9 +16,9 @@ const ROOT = path.resolve(import.meta.dir, '..'); let home: string; let remote: string; -function run(argv: string[]) { +function run(argv: string[], extraEnv: Record = {}) { const r = spawnSync(path.join(ROOT, 'bin', argv[0]), argv.slice(1), { - env: { ...process.env, HOME: home, GSTACK_HOME: home }, + env: { ...process.env, HOME: home, GSTACK_HOME: home, ...extraEnv }, encoding: 'utf-8', timeout: 60_000, }); @@ -145,6 +145,24 @@ describe('artifacts_sync_removals', () => { expect(run(['gstack-brain-sync', '--publish-removals', '--yes']).stdout).toContain('no removals to publish'); }); + test('--publish-removals cleans up its temp files and lock on every exit path', () => { + syncAll(['projects/p/designs/a.md', 'projects/p/designs/b.md']); + fs.rmSync(path.join(home, 'projects/p/designs/a.md')); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'brain-removals-tmp-')); + try { + for (const argv of [['--publish-removals'], ['--publish-removals', '--yes'], ['--publish-removals', '--yes']]) { + const r = run(['gstack-brain-sync', ...argv], { TMPDIR: tmp }); + expect(r.stderr).not.toContain('unbound variable'); + expect(r.status).toBe(0); + expect(fs.readdirSync(tmp)).toEqual([]); + expect(fs.existsSync(path.join(home, '.brain-sync.lock.d'))).toBe(false); + } + expect(remoteFiles()).toEqual(['projects/p/designs/b.md']); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + test('config rejects values other than on and off', () => { const r = run(['gstack-config', 'set', 'artifacts_sync_removals', 'yes']); expect(r.stderr).toContain("artifacts_sync_removals 'yes' not recognized");