mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-11 03:00:26 +01:00
Stop gstack-uninstall deleting nested worktrees, locked worktrees and a .gstack repo (GST-2268) (#4267)
* Stop gstack-uninstall deleting nested worktrees, locked worktrees and a .gstack repo (GST-2268) The worktree check now walks every checkout under .gstack-worktrees/, nested ones included, and keeps a submodule of a clean lane covered by that lane's status. A locked worktree stops the uninstall even with --force-worktrees. A .gstack/ that holds its own git repository is kept with a message. The check runs again right before .gstack-worktrees/ is deleted, so work saved while the prompt waited is kept (exit 2). The manual steps in docs/uninstall.md no longer rm -rf the per-project folders. test/uninstall.test.ts is already excluded from the Windows subset by scripts/lib/windows-curation.ts (it spawns a bin/ shebang script). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep a lane's submodule when it holds commits no remote-tracking branch has (GST-2268) The submodule exemption trusted the lane's status, which only compares the submodule's HEAD to the recorded gitlink. Commits on the submodule's own branches or stash live in the lane's git dir and were deleted with exit 0. Uninstall now refuses such a submodule, and matches the gitlink by exact path instead of a prefix or glob pathspec. Review-Finding: s11/outside-voice#1 reader=outside-voice Review-Finding: s11/adversarial-native#1 reader=adversarial-native Review-Finding: p1/specialist-batched#1 reader=testing Review-Finding: p1/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Explain the submodule rule in the uninstall docs (GST-2268) docs/uninstall.md now says a submodule inside a clean lane goes only when its commits are on a remote-tracking branch, and the changelog entry drops a mid-sentence colon. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Make every gstack-uninstall keep check run before anything is deleted (GST-2268) The post-prompt worktree check now runs right after confirmation, before the install root or anything else is removed, so running uninstall from inside the gstack install no longer deletes a worktree edited during the prompt. A .gstack/ holding its own git repository inside the install being removed now stops the uninstall. A failing git worktree list stops it too, even with --force-worktrees. The manual cleanup in docs/uninstall.md now matches worktree paths by prefix, so it never removes a worktree outside the project whose path merely contains it. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Also stop gstack-uninstall when a .gstack repo sits inside a Cursor, OpenCode, Copilot, Antigravity or Gemini install (GST-2268) Review-Finding: p1/specialist-batched#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Split a run-on sentence in the uninstall release note (GST-2268) 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
4685fa9233
commit
6c23638d6d
4 files changed
+417
-33
No files matched your search
+266
-1
@@ -1,5 +1,5 @@
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import { spawn, spawnSync } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as path from 'path';
|
||||
import * as os from 'os';
|
||||
@@ -827,6 +827,271 @@ describe('per-project state: tracked files and dirty worktrees survive (GSTA-461
|
||||
expect(fs.existsSync(path.join(repo, worktree))).toBe(true);
|
||||
});
|
||||
|
||||
test('a symlink inside .gstack-worktrees to a repo outside is only unlinked by --force --force-worktrees', () => {
|
||||
const outside = path.join(tmp, 'outside');
|
||||
fs.mkdirSync(outside);
|
||||
git(['init', '-q', '-b', 'main'], outside);
|
||||
fs.writeFileSync(path.join(outside, 'keep.txt'), 'keep\n');
|
||||
fs.mkdirSync(path.join(repo, '.gstack-worktrees', 'run-1'), { recursive: true });
|
||||
fs.symlinkSync(outside, path.join(repo, '.gstack-worktrees', 'run-1', 'lane'));
|
||||
const r = uninstall(['--force', '--force-worktrees']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack-worktrees'))).toBe(false);
|
||||
expect(fs.readFileSync(path.join(outside, 'keep.txt'), 'utf-8')).toBe('keep\n');
|
||||
expect(fs.existsSync(path.join(outside, '.git', 'HEAD'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a dirty worktree nested inside a clean lane stops the uninstall and keeps its edit', () => {
|
||||
addWorktree(false);
|
||||
const inner = path.join(repo, worktree, '.gstack-worktrees', 'run-2', 'inner');
|
||||
git(['worktree', 'add', '-q', '--detach', inner, 'HEAD'], path.join(repo, worktree));
|
||||
fs.writeFileSync(path.join(inner, 'README.md'), 'unsaved edit\n');
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${inner}: it has uncommitted changes`);
|
||||
expect(fs.readFileSync(path.join(inner, 'README.md'), 'utf-8')).toBe('unsaved edit\n');
|
||||
expect(fs.existsSync(path.join(home, '.claude', 'skills', 'gstack'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a clean worktree nested inside a clean lane is removed with its registration', () => {
|
||||
addWorktree(false);
|
||||
const inner = path.join(repo, worktree, '.gstack-worktrees', 'run-2', 'inner');
|
||||
git(['worktree', 'add', '-q', '--detach', inner, 'HEAD'], path.join(repo, worktree));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack-worktrees'))).toBe(false);
|
||||
expect(git(['worktree', 'list', '--porcelain'])).not.toContain('.gstack-worktrees');
|
||||
});
|
||||
|
||||
test('an initialized submodule inside a clean lane does not stop the uninstall', () => {
|
||||
const lib = path.join(tmp, 'lib');
|
||||
fs.mkdirSync(lib);
|
||||
git(['init', '-q', '-b', 'main'], lib);
|
||||
fs.writeFileSync(path.join(lib, 'lib.txt'), 'lib\n');
|
||||
git(['add', 'lib.txt'], lib);
|
||||
git(['commit', '-q', '-m', 'lib'], lib);
|
||||
git(['-c', 'protocol.file.allow=always', 'submodule', 'add', '-q', lib, 'vendor/lib']);
|
||||
git(['commit', '-q', '-m', 'submodule']);
|
||||
addWorktree(false);
|
||||
git(['-c', 'protocol.file.allow=always', 'submodule', 'update', '-q', '--init'], path.join(repo, worktree));
|
||||
expect(fs.existsSync(path.join(repo, worktree, 'vendor', 'lib', '.git'))).toBe(true);
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.stderr).toBe('');
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack-worktrees'))).toBe(false);
|
||||
});
|
||||
|
||||
test('a submodule inside a clean lane with a commit no remote-tracking branch holds stops the uninstall', () => {
|
||||
const lib = path.join(tmp, 'lib');
|
||||
fs.mkdirSync(lib);
|
||||
git(['init', '-q', '-b', 'main'], lib);
|
||||
fs.writeFileSync(path.join(lib, 'lib.txt'), 'lib\n');
|
||||
git(['add', 'lib.txt'], lib);
|
||||
git(['commit', '-q', '-m', 'lib'], lib);
|
||||
git(['-c', 'protocol.file.allow=always', 'submodule', 'add', '-q', lib, 'vendor/lib']);
|
||||
git(['commit', '-q', '-m', 'submodule']);
|
||||
addWorktree(false);
|
||||
git(['-c', 'protocol.file.allow=always', 'submodule', 'update', '-q', '--init'], path.join(repo, worktree));
|
||||
const sub = path.join(repo, worktree, 'vendor', 'lib');
|
||||
const recorded = git(['rev-parse', 'HEAD'], sub).trim();
|
||||
git(['switch', '-q', '-c', 'wip'], sub);
|
||||
fs.writeFileSync(path.join(sub, 'wip.txt'), 'local only\n');
|
||||
git(['add', 'wip.txt'], sub);
|
||||
git(['commit', '-q', '-m', 'wip'], sub);
|
||||
const wip = git(['rev-parse', 'HEAD'], sub).trim();
|
||||
git(['switch', '-q', '--detach', recorded], sub);
|
||||
expect(git(['status', '--porcelain', '--ignore-submodules=none'], path.join(repo, worktree))).toBe('');
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${sub}: it is a submodule with commits no remote-tracking branch contains`);
|
||||
expect(git(['cat-file', '-t', wip], sub).trim()).toBe('commit');
|
||||
});
|
||||
|
||||
for (const flags of [['--force'], ['--force', '--force-worktrees']]) {
|
||||
test(`a locked worktree stops ${flags.join(' ')} and is kept with its registration`, () => {
|
||||
addWorktree(false);
|
||||
git(['worktree', 'lock', '--reason', 'on a USB disk', worktree]);
|
||||
const r = uninstall(flags);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${path.join(repo, worktree)}: it is locked`);
|
||||
expect(r.stderr).toMatch(/^fix: .*git worktree unlock/m);
|
||||
expect(fs.existsSync(path.join(repo, worktree, 'README.md'))).toBe(true);
|
||||
expect(git(['worktree', 'list', '--porcelain'])).toContain(`worktree ${path.join(repo, worktree)}\n`);
|
||||
});
|
||||
}
|
||||
|
||||
test('a .gstack that is its own git repo is kept, with the reason, when the project tracks nothing there', () => {
|
||||
git(['init', '-q', '-b', 'main'], path.join(repo, '.gstack'));
|
||||
fs.writeFileSync(path.join(repo, '.gstack', 'notes.md'), 'mine\n');
|
||||
git(['add', 'notes.md'], path.join(repo, '.gstack'));
|
||||
git(['commit', '-q', '-m', 'unpushed notes'], path.join(repo, '.gstack'));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stderr).toContain(`kept ${repo}/.gstack/: it holds its own git repository`);
|
||||
expect(git(['log', '--format=%s'], path.join(repo, '.gstack'))).toBe('unpushed notes\n');
|
||||
expect(fs.existsSync(path.join(home, '.gstack'))).toBe(false);
|
||||
});
|
||||
|
||||
test('a worktree edited while the prompt waits is kept: the check runs again before deleting', async () => {
|
||||
addWorktree(false);
|
||||
const child = spawn('bash', [UNINSTALL], {
|
||||
cwd: repo, timeout: 30_000, stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: { ...gitEnv, HOME: home, GSTACK_DIR: path.join(home, '.claude', 'skills', 'gstack'),
|
||||
GSTACK_SETTINGS_FILE: path.join(home, 'settings.json'),
|
||||
GSTACK_HOME: '', GSTACK_STATE_ROOT: path.join(home, '.gstack'), GSTACK_STATE_DIR: '', CLAUDE_PLUGIN_DATA: '' },
|
||||
});
|
||||
let stdout = '', stderr = '';
|
||||
child.stderr.on('data', (d) => { stderr += d; });
|
||||
child.stdout.on('data', (d) => {
|
||||
stdout += d;
|
||||
if (stdout.includes('Continue? [y/N]') && !child.stdin.writableEnded) {
|
||||
fs.writeFileSync(path.join(repo, worktree, 'wip.txt'), 'typed during the prompt\n');
|
||||
child.stdin.end('y\n');
|
||||
}
|
||||
});
|
||||
const status = await new Promise<number | null>((resolve) => child.on('close', resolve));
|
||||
expect(status).toBe(2);
|
||||
expect(stderr).toContain(`refusing to delete ${path.join(repo, worktree)}: it has uncommitted changes`);
|
||||
expect(stderr).toContain('nothing was removed: a checkout changed while the prompt waited');
|
||||
expect(fs.existsSync(path.join(home, '.claude', 'skills', 'gstack'))).toBe(true);
|
||||
expect(fs.readFileSync(path.join(repo, worktree, 'wip.txt'), 'utf-8')).toBe('typed during the prompt\n');
|
||||
});
|
||||
|
||||
const promptRun = (cwd: string, onPrompt: () => void) => new Promise<{ status: number | null; stderr: string }>((resolve) => {
|
||||
const child = spawn('bash', [UNINSTALL], {
|
||||
cwd, timeout: 30_000, stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: { ...gitEnv, HOME: home, GSTACK_DIR: path.join(home, '.claude', 'skills', 'gstack'),
|
||||
GSTACK_SETTINGS_FILE: path.join(home, 'settings.json'),
|
||||
GSTACK_HOME: '', GSTACK_STATE_ROOT: path.join(home, '.gstack'), GSTACK_STATE_DIR: '', CLAUDE_PLUGIN_DATA: '' },
|
||||
});
|
||||
let stdout = '', stderr = '';
|
||||
child.stderr.on('data', (d) => { stderr += d; });
|
||||
child.stdout.on('data', (d) => {
|
||||
stdout += d;
|
||||
if (stdout.includes('Continue? [y/N]') && !child.stdin.writableEnded) { onPrompt(); child.stdin.end('y\n'); }
|
||||
});
|
||||
child.on('close', (status) => resolve({ status, stderr }));
|
||||
});
|
||||
const makeInstallRepo = () => {
|
||||
const install = path.join(home, '.claude', 'skills', 'gstack');
|
||||
git(['init', '-q', '-b', 'main'], install);
|
||||
fs.writeFileSync(path.join(install, '.git', 'info', 'exclude'), '.gstack-worktrees/\n.gstack/\n');
|
||||
git(['add', 'SKILL.md'], install);
|
||||
git(['commit', '-q', '-m', 'install'], install);
|
||||
return install;
|
||||
};
|
||||
|
||||
test('run from inside the install, a worktree edited during the prompt stops the uninstall before anything is removed', async () => {
|
||||
const install = makeInstallRepo();
|
||||
const lane = path.join(install, '.gstack-worktrees', 'run-1', 'lane');
|
||||
git(['worktree', 'add', '-q', '--detach', lane, 'HEAD'], install);
|
||||
const r = await promptRun(install, () => fs.writeFileSync(path.join(lane, 'wip.txt'), 'typed during the prompt\n'));
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${lane}: it has uncommitted changes`);
|
||||
expect(fs.readFileSync(path.join(lane, 'wip.txt'), 'utf-8')).toBe('typed during the prompt\n');
|
||||
expect(fs.existsSync(path.join(install, 'SKILL.md'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(home, '.gstack'))).toBe(true);
|
||||
});
|
||||
|
||||
test('run from inside the install, a .gstack that is its own repo stops the uninstall before anything is removed', () => {
|
||||
const install = makeInstallRepo();
|
||||
const own = path.join(install, '.gstack');
|
||||
fs.mkdirSync(own);
|
||||
git(['init', '-q', '-b', 'main'], own);
|
||||
fs.writeFileSync(path.join(own, 'notes.md'), 'mine\n');
|
||||
git(['add', 'notes.md'], own);
|
||||
git(['commit', '-q', '-m', 'unpushed notes'], own);
|
||||
const r = spawnSync('bash', [UNINSTALL, '--force'], {
|
||||
cwd: install, encoding: 'utf-8', timeout: 30_000,
|
||||
env: { ...gitEnv, HOME: home, GSTACK_DIR: install, GSTACK_SETTINGS_FILE: path.join(home, 'settings.json'),
|
||||
GSTACK_HOME: '', GSTACK_STATE_ROOT: path.join(home, '.gstack'), GSTACK_STATE_DIR: '', CLAUDE_PLUGIN_DATA: '' },
|
||||
});
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`${install}/.gstack/ holds its own git repository`);
|
||||
expect(git(['log', '--format=%s'], own)).toBe('unpushed notes\n');
|
||||
expect(fs.existsSync(path.join(home, '.gstack'))).toBe(true);
|
||||
});
|
||||
|
||||
test('run from inside a Cursor install, a .gstack that is its own repo stops the uninstall before anything is removed', () => {
|
||||
const install = path.join(home, '.cursor', 'skills', 'gstack');
|
||||
fs.mkdirSync(install, { recursive: true });
|
||||
fs.writeFileSync(path.join(install, 'SKILL.md'), '<!-- AUTO-GENERATED from SKILL.md.tmpl -->\n');
|
||||
git(['init', '-q', '-b', 'main'], install);
|
||||
fs.writeFileSync(path.join(install, '.git', 'info', 'exclude'), '.gstack/\n');
|
||||
git(['add', 'SKILL.md'], install);
|
||||
git(['commit', '-q', '-m', 'install'], install);
|
||||
const own = path.join(install, '.gstack');
|
||||
fs.mkdirSync(own);
|
||||
git(['init', '-q', '-b', 'main'], own);
|
||||
fs.writeFileSync(path.join(own, 'notes.md'), 'mine\n');
|
||||
git(['add', 'notes.md'], own);
|
||||
git(['commit', '-q', '-m', 'unpushed notes'], own);
|
||||
const r = spawnSync('bash', [UNINSTALL, '--force'], {
|
||||
cwd: install, encoding: 'utf-8', timeout: 30_000,
|
||||
env: { ...gitEnv, HOME: home, GSTACK_DIR: path.join(home, '.claude', 'skills', 'gstack'), GSTACK_SETTINGS_FILE: path.join(home, 'settings.json'),
|
||||
GSTACK_HOME: '', GSTACK_STATE_ROOT: path.join(home, '.gstack'), GSTACK_STATE_DIR: '', CLAUDE_PLUGIN_DATA: '' },
|
||||
});
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`${install}/.gstack/ holds its own git repository`);
|
||||
expect(git(['log', '--format=%s'], own)).toBe('unpushed notes\n');
|
||||
expect(fs.existsSync(path.join(install, 'SKILL.md'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a failing git worktree list stops --force --force-worktrees and keeps the checkouts', () => {
|
||||
addWorktree(true);
|
||||
const realGit = spawnSync('bash', ['-c', 'command -v git'], { encoding: 'utf-8', timeout: 30_000 }).stdout.trim();
|
||||
const shim = path.join(tmp, 'shim');
|
||||
fs.mkdirSync(shim);
|
||||
fs.writeFileSync(path.join(shim, 'git'), `#!/bin/sh\ncase " $* " in *" worktree list "*) echo "fatal: broken" >&2; exit 128 ;; esac\nexec '${realGit}' "$@"\n`, { mode: 0o755 });
|
||||
const r = spawnSync('bash', [UNINSTALL, '--force', '--force-worktrees'], {
|
||||
cwd: repo, encoding: 'utf-8', timeout: 30_000,
|
||||
env: { ...gitEnv, PATH: `${shim}:${process.env.PATH}`, HOME: home, GSTACK_DIR: path.join(home, '.claude', 'skills', 'gstack'),
|
||||
GSTACK_SETTINGS_FILE: path.join(home, 'settings.json'),
|
||||
GSTACK_HOME: '', GSTACK_STATE_ROOT: path.join(home, '.gstack'), GSTACK_STATE_DIR: '', CLAUDE_PLUGIN_DATA: '' },
|
||||
});
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain("git cannot list this repository's worktrees");
|
||||
expect(fs.readFileSync(path.join(repo, worktree, 'wip.txt'), 'utf-8')).toBe('unsaved work\n');
|
||||
expect(fs.existsSync(path.join(home, '.claude', 'skills', 'gstack'))).toBe(true);
|
||||
});
|
||||
|
||||
test('the manual cleanup in docs/uninstall.md never touches a worktree outside the project whose path contains it', () => {
|
||||
const doc = fs.readFileSync(path.join(ROOT, 'docs', 'uninstall.md'), 'utf-8');
|
||||
const start = doc.indexOf('# 7. Per-project cleanup');
|
||||
const block = doc.slice(start, doc.indexOf('\n```', start));
|
||||
const external = path.join(tmp, 'mirror') + path.join(repo, '.gstack-worktrees', 'run-1', 'lane');
|
||||
git(['worktree', 'add', '-q', '--detach', external, 'HEAD']);
|
||||
fs.appendFileSync(path.join(repo, '.git', 'info', 'exclude'), 'personal.txt\n');
|
||||
fs.writeFileSync(path.join(external, 'personal.txt'), 'mine\n');
|
||||
const r = spawnSync('bash', ['-c', block], { cwd: repo, encoding: 'utf-8', timeout: 30_000, env: gitEnv });
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.readFileSync(path.join(external, 'personal.txt'), 'utf-8')).toBe('mine\n');
|
||||
expect(git(['worktree', 'list', '--porcelain'])).toContain(`worktree ${external}\n`);
|
||||
});
|
||||
|
||||
test('the manual per-project cleanup in docs/uninstall.md keeps dirty and nested worktrees and tracked policy', () => {
|
||||
const doc = fs.readFileSync(path.join(ROOT, 'docs', 'uninstall.md'), 'utf-8');
|
||||
const start = doc.indexOf('# 7. Per-project cleanup');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const block = doc.slice(start, doc.indexOf('\n```', start));
|
||||
trackPolicyAndAddBrowseState();
|
||||
addWorktree(true);
|
||||
const lane = path.join(repo, '.gstack-worktrees', 'run-1', 'lane-2');
|
||||
git(['worktree', 'add', '-q', '--detach', lane, 'HEAD']);
|
||||
const inner = path.join(lane, '.gstack-worktrees', 'run-2', 'inner');
|
||||
git(['worktree', 'add', '-q', '--detach', inner, 'HEAD'], lane);
|
||||
fs.writeFileSync(path.join(inner, 'README.md'), 'unsaved edit\n');
|
||||
const clean = path.join(repo, '.gstack-worktrees', 'run-3', 'lane');
|
||||
git(['worktree', 'add', '-q', '--detach', clean, 'HEAD']);
|
||||
const r = spawnSync('bash', ['-c', block], { cwd: repo, encoding: 'utf-8', timeout: 30_000, env: gitEnv });
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.readFileSync(path.join(repo, worktree, 'wip.txt'), 'utf-8')).toBe('unsaved work\n');
|
||||
expect(fs.readFileSync(path.join(inner, 'README.md'), 'utf-8')).toBe('unsaved edit\n');
|
||||
expect(fs.existsSync(clean)).toBe(false);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack', 'ship-policy.json'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack', 'browse.json'))).toBe(false);
|
||||
});
|
||||
|
||||
test('hold: a plain uninstall still removes untracked .gstack state and clean worktrees entirely', () => {
|
||||
fs.writeFileSync(path.join(repo, '.gstack', 'browse.json'), '{}\n');
|
||||
fs.mkdirSync(path.join(repo, '.gstack', 'reports'), { recursive: true });
|
||||
|
||||
Reference in new issue
Block a user