mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 18:50:18 +01:00
Stop gstack-uninstall deleting a repo's committed .gstack files and unsaved worktrees (GST-2197) (#4241)
* Stop gstack-uninstall deleting a repo's committed .gstack files and unsaved worktrees (GST-2197) --keep-state now keeps the repo's .gstack/ and .gstack-worktrees/. A plain uninstall keeps git-tracked .gstack files, removes only ignored state, and stops before removing anything when a worktree has uncommitted changes unless --force-worktrees is given. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep moved, config-hidden and committed worktree work from gstack-uninstall (GST-2197) The dirty-worktree guard now checks every checkout on disk under .gstack-worktrees/, pins git status flags so config cannot hide new files, refuses a checkout whose commits no branch or tag holds, and keeps .gstack/ when git cannot list its tracked files. Review-Finding: p1/red-team#2 reader=red-team Review-Finding: p1/red-team#3 reader=red-team Review-Finding: p1/specialist-batched#1 reader=security Review-Finding: p1/red-team#4 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Make gstack-uninstall list worktree checkouts safely and never run their config (GST-2197) Checkouts are listed NUL-separated and through a symlinked .gstack-worktrees, only worktrees this repository registered are opened with git, and those git calls turn off fsmonitor, hooks and optional index writes. Review-Finding: v1/adversarial-native#1 reader=adversarial-native Review-Finding: v1/adversarial-native#5 reader=adversarial-native Review-Finding: v1/adversarial-native#2 reader=adversarial-native Review-Loop-Grant: a-f3b2ff8e662e Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Check a .gstack-worktrees that is itself a git repo before gstack-uninstall deletes it (GST-2197) Review-Finding: fc1/fix-check#1 reader=fix-check Review-Loop-Grant: a-3b80e7929588 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Document what gstack-uninstall keeps and refuses, and the new --force-worktrees flag (GST-2197) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Stop gstack-uninstall dropping the registration of worktrees it did not delete (GST-2197) Uninstall ran a repo-wide git worktree prune, which unregistered every worktree whose folder was missing at that moment, including ones outside .gstack-worktrees on an unmounted drive, and left their detached commits unreachable. It now unregisters only the gstack checkouts it just removed. Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say that gstack-uninstall does not yet check worktrees nested inside another checkout (GST-2197) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Stop gstack-uninstall deleting checkouts through a .gstack-worktrees symlink that points outside the repo (GST-2197) Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/red-team#2 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Make gstack-uninstall remove only the link when .gstack-worktrees is a symlink, as before (GST-2197) Review-Finding: p2/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Drop release-note wording from the gstack-uninstall docs (GST-2197) 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
3183120cec
commit
490bbd5202
4 files changed
+353
-14
No files matched your search
@@ -601,3 +601,243 @@ describe('state-root safety: only the default root is deleted (docs/state-root.m
|
||||
expect(fs.lstatSync(path.join(mockHome, '.gstack')).isSymbolicLink()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// Value: protects=a repo's tracked .gstack/ policy files and the uncommitted work in its .gstack-worktrees/ checkouts;
|
||||
// fails_when=uninstall removes per-project state without checking --keep-state, git-tracked files, or dirty worktrees;
|
||||
// why_new=GSTA-461 item 9 (data loss under --keep-state); seam=none; no_claim=a real gstack install (temp HOME and state root)
|
||||
describe('per-project state: tracked files and dirty worktrees survive (GSTA-461 item 9)', () => {
|
||||
let tmp: string, home: string, repo: string, gitEnv: Record<string, string>;
|
||||
const git = (args: string[], cwd = repo) => {
|
||||
const r = spawnSync('git', ['-c', 'user.name=fixture', '-c', 'user.email=fixture@example.com', '-c', 'commit.gpgsign=false', ...args], { cwd, encoding: 'utf-8', timeout: 30_000, env: gitEnv });
|
||||
expect(r.status).toBe(0);
|
||||
return r.stdout;
|
||||
};
|
||||
const uninstall = (args: string[]) => spawnSync('bash', [UNINSTALL, ...args], {
|
||||
cwd: repo, 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: '' },
|
||||
});
|
||||
const worktree = path.join('.gstack-worktrees', 'run-1', 'lane-1');
|
||||
|
||||
beforeEach(() => {
|
||||
tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-uninstall-proj-')));
|
||||
home = path.join(tmp, 'home');
|
||||
repo = path.join(tmp, 'proj');
|
||||
fs.writeFileSync(path.join(tmp, 'gitconfig'), '');
|
||||
gitEnv = { ...process.env, GIT_CONFIG_GLOBAL: path.join(tmp, 'gitconfig'), GIT_CONFIG_NOSYSTEM: '1' } as Record<string, string>;
|
||||
fs.mkdirSync(path.join(home, '.claude', 'skills', 'gstack'), { recursive: true });
|
||||
fs.writeFileSync(path.join(home, '.claude', 'skills', 'gstack', 'SKILL.md'), 'test');
|
||||
fs.mkdirSync(path.join(home, '.gstack'), { recursive: true });
|
||||
fs.mkdirSync(path.join(repo, '.gstack'), { recursive: true });
|
||||
git(['init', '-q', '-b', 'main']);
|
||||
fs.writeFileSync(path.join(repo, '.git', 'info', 'exclude'), '.gstack-worktrees/\n');
|
||||
fs.writeFileSync(path.join(repo, 'README.md'), 'x\n');
|
||||
git(['add', 'README.md']);
|
||||
git(['commit', '-q', '-m', 'seed']);
|
||||
});
|
||||
afterEach(() => fs.rmSync(tmp, { recursive: true, force: true }));
|
||||
|
||||
function trackPolicyAndAddBrowseState() {
|
||||
fs.writeFileSync(path.join(repo, '.gstack', 'ship-policy.json'), '{"merge":"direct"}\n');
|
||||
git(['add', '.gstack/ship-policy.json']);
|
||||
git(['commit', '-q', '-m', 'policy']);
|
||||
fs.writeFileSync(path.join(repo, '.gstack', '.gitignore'), '*\n');
|
||||
fs.writeFileSync(path.join(repo, '.gstack', 'browse.json'), '{}\n');
|
||||
}
|
||||
function addWorktree(dirty: boolean) {
|
||||
git(['worktree', 'add', '-q', '--detach', worktree, 'HEAD']);
|
||||
if (dirty) fs.writeFileSync(path.join(repo, worktree, 'wip.txt'), 'unsaved work\n');
|
||||
}
|
||||
|
||||
test('--keep-state keeps the repo .gstack/ and every .gstack-worktrees/ checkout', () => {
|
||||
trackPolicyAndAddBrowseState();
|
||||
addWorktree(true);
|
||||
const r = uninstall(['--force', '--keep-state']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).not.toContain(`${repo}/.gstack`);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack', 'browse.json'))).toBe(true);
|
||||
expect(git(['status', '--short'])).toBe('');
|
||||
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(false);
|
||||
});
|
||||
|
||||
test('a plain uninstall keeps git-tracked .gstack files and removes the ignored browse state', () => {
|
||||
trackPolicyAndAddBrowseState();
|
||||
fs.writeFileSync(path.join(repo, '.gstack', 'ship-policy.json'), '{"merge":"queue"}\n');
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.readFileSync(path.join(repo, '.gstack', 'ship-policy.json'), 'utf-8')).toBe('{"merge":"queue"}\n');
|
||||
expect(fs.existsSync(path.join(repo, '.gstack', 'browse.json'))).toBe(false);
|
||||
expect(git(['status', '--short', '--', '.gstack'])).toBe(' M .gstack/ship-policy.json\n');
|
||||
expect(r.stderr).toContain('kept git-tracked files');
|
||||
});
|
||||
|
||||
test('a dirty worktree stops a plain uninstall before anything is removed, naming the worktree and the fix', () => {
|
||||
trackPolicyAndAddBrowseState();
|
||||
addWorktree(true);
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${path.join(repo, worktree)}: it has uncommitted changes`);
|
||||
expect(r.stderr).toMatch(/^fix: .*--keep-state.*--force-worktrees/m);
|
||||
expect(fs.readFileSync(path.join(repo, worktree, 'wip.txt'), 'utf-8')).toBe('unsaved work\n');
|
||||
expect(fs.existsSync(path.join(repo, '.gstack', 'browse.json'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(home, '.claude', 'skills', 'gstack'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(home, '.gstack'))).toBe(true);
|
||||
});
|
||||
|
||||
test('--force-worktrees removes a dirty worktree and prunes its registration', () => {
|
||||
addWorktree(true);
|
||||
const r = uninstall(['--force', '--force-worktrees']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack-worktrees'))).toBe(false);
|
||||
expect(git(['worktree', 'list', '--porcelain'])).not.toContain('lane-1');
|
||||
});
|
||||
|
||||
test('a missing worktree outside .gstack-worktrees keeps its registration and its commits', () => {
|
||||
addWorktree(false);
|
||||
const away = path.join(tmp, 'usb', 'wt');
|
||||
git(['worktree', 'add', '-q', '--detach', away, 'HEAD']);
|
||||
git(['commit', '-q', '--allow-empty', '-m', 'unique detached work'], away);
|
||||
const head = git(['rev-parse', 'HEAD'], away).trim();
|
||||
fs.renameSync(path.join(tmp, 'usb'), path.join(tmp, 'usb.unmounted'));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack-worktrees'))).toBe(false);
|
||||
const list = git(['worktree', 'list', '--porcelain']);
|
||||
expect(list).toContain(`worktree ${away}\nHEAD ${head}\n`);
|
||||
expect(list).not.toContain('lane-1');
|
||||
});
|
||||
|
||||
test('status.showUntrackedFiles=no does not hide a new file in a worktree', () => {
|
||||
addWorktree(true);
|
||||
git(['config', 'status.showUntrackedFiles', 'no']);
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${path.join(repo, worktree)}: it has uncommitted changes`);
|
||||
expect(fs.existsSync(path.join(repo, worktree, 'wip.txt'))).toBe(true);
|
||||
});
|
||||
|
||||
test('a dirty worktree still stops the uninstall after the repo is moved', () => {
|
||||
addWorktree(true);
|
||||
const moved = path.join(tmp, 'moved');
|
||||
fs.renameSync(repo, moved);
|
||||
repo = moved;
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${path.join(moved, worktree)}: it is not a registered worktree of this repository`);
|
||||
expect(fs.readFileSync(path.join(moved, worktree, 'wip.txt'), 'utf-8')).toBe('unsaved work\n');
|
||||
});
|
||||
|
||||
test('a checkout name with a newline never turns into a relative delete', () => {
|
||||
fs.mkdirSync(path.join(repo, 'src'), { recursive: true });
|
||||
fs.writeFileSync(path.join(repo, 'src', 'app.txt'), 'app\n');
|
||||
git(['add', 'src/app.txt']);
|
||||
git(['commit', '-q', '-m', 'src']);
|
||||
fs.mkdirSync(path.join(repo, '.gstack-worktrees', 'x\nsrc', '.git'), { recursive: true });
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain('refusing to delete');
|
||||
expect(fs.readFileSync(path.join(repo, 'src', 'app.txt'), 'utf-8')).toBe('app\n');
|
||||
});
|
||||
|
||||
const linkGone = () => {
|
||||
try { fs.lstatSync(path.join(repo, '.gstack-worktrees')); return false; } catch { return true; }
|
||||
};
|
||||
|
||||
test('a symlinked .gstack-worktrees is unlinked, never followed, even when a checkout behind it has unique commits', () => {
|
||||
const store = path.join(repo, 'worktree-store');
|
||||
fs.mkdirSync(store);
|
||||
fs.symlinkSync(store, path.join(repo, '.gstack-worktrees'));
|
||||
addWorktree(false);
|
||||
git(['commit', '-q', '--allow-empty', '-m', 'wip'], path.join(repo, worktree));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(linkGone()).toBe(true);
|
||||
expect(fs.existsSync(path.join(store, 'run-1', 'lane-1', 'README.md'))).toBe(true);
|
||||
expect(git(['worktree', 'list', '--porcelain'])).toContain(`worktree ${path.join(store, 'run-1', 'lane-1')}\n`);
|
||||
});
|
||||
|
||||
test('a .gstack-worktrees symlink to a folder inside the repo keeps the worktrees behind it and their ignored files', () => {
|
||||
fs.appendFileSync(path.join(repo, '.git', 'info', 'exclude'), '.claude/worktrees/\n.env\n');
|
||||
const feat = path.join(repo, '.claude', 'worktrees', 'feat');
|
||||
git(['worktree', 'add', '-q', '-b', 'feat', feat]);
|
||||
fs.writeFileSync(path.join(feat, '.env'), 'SECRET=1\n');
|
||||
fs.symlinkSync('.claude/worktrees', path.join(repo, '.gstack-worktrees'));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(linkGone()).toBe(true);
|
||||
expect(fs.readFileSync(path.join(feat, '.env'), 'utf-8')).toBe('SECRET=1\n');
|
||||
expect(git(['worktree', 'list', '--porcelain'])).toContain(`worktree ${feat}\n`);
|
||||
});
|
||||
|
||||
for (const flags of [['--force'], ['--force', '--force-worktrees']]) {
|
||||
test(`a .gstack-worktrees symlink to a folder outside the repo is only unlinked by ${flags.join(' ')}`, () => {
|
||||
const store = path.join(tmp, 'worktree-store');
|
||||
fs.mkdirSync(store);
|
||||
git(['worktree', 'add', '-q', '-b', 'feature', path.join(store, 'feature')]);
|
||||
fs.symlinkSync(store, path.join(repo, '.gstack-worktrees'));
|
||||
const r = uninstall(flags);
|
||||
expect(r.status).toBe(0);
|
||||
expect(linkGone()).toBe(true);
|
||||
expect(fs.existsSync(path.join(store, 'feature', 'README.md'))).toBe(true);
|
||||
expect(git(['worktree', 'list', '--porcelain'])).toContain(`worktree ${path.join(store, 'feature')}\n`);
|
||||
expect(fs.existsSync(path.join(home, '.claude', 'skills', 'gstack'))).toBe(false);
|
||||
});
|
||||
}
|
||||
|
||||
test('a .gstack-worktrees symlink to a folder holding the main checkout never deletes the repo', () => {
|
||||
fs.symlinkSync(tmp, path.join(repo, '.gstack-worktrees'));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(linkGone()).toBe(true);
|
||||
expect(fs.existsSync(path.join(repo, '.git', 'HEAD'))).toBe(true);
|
||||
expect(fs.readFileSync(path.join(repo, 'README.md'), 'utf-8')).toBe('x\n');
|
||||
});
|
||||
|
||||
test('a repo under .gstack-worktrees never runs its own config commands during the check', () => {
|
||||
const nested = path.join(repo, '.gstack-worktrees', 'run-1', 'clone');
|
||||
fs.mkdirSync(nested, { recursive: true });
|
||||
git(['init', '-q', '-b', 'main'], nested);
|
||||
fs.writeFileSync(path.join(nested, 'a.txt'), 'a\n');
|
||||
git(['add', 'a.txt'], nested);
|
||||
git(['commit', '-q', '-m', 'a'], nested);
|
||||
const marker = path.join(tmp, 'fsmonitor-ran');
|
||||
git(['config', 'core.fsmonitor', `touch '${marker}'; false`], nested);
|
||||
const r = uninstall(['--force']);
|
||||
expect(fs.existsSync(marker)).toBe(false);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${nested}: it is not a registered worktree of this repository`);
|
||||
});
|
||||
|
||||
test('a .gstack-worktrees that is itself a git repo stops the uninstall and keeps its work', () => {
|
||||
git(['init', '-q', '.gstack-worktrees']);
|
||||
fs.writeFileSync(path.join(repo, '.gstack-worktrees', 'wip.txt'), 'unsaved work\n');
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(fs.readFileSync(path.join(repo, '.gstack-worktrees', 'wip.txt'), 'utf-8')).toBe('unsaved work\n');
|
||||
});
|
||||
|
||||
test('a clean worktree holding a commit no branch contains stops the uninstall', () => {
|
||||
addWorktree(false);
|
||||
git(['commit', '-q', '--allow-empty', '-m', 'wip'], path.join(repo, worktree));
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(2);
|
||||
expect(r.stderr).toContain(`refusing to delete ${path.join(repo, worktree)}: it has commits no branch or tag contains`);
|
||||
expect(fs.existsSync(path.join(repo, worktree))).toBe(true);
|
||||
});
|
||||
|
||||
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 });
|
||||
fs.writeFileSync(path.join(repo, '.gstack', 'reports', 'qa.md'), '# qa\n');
|
||||
addWorktree(false);
|
||||
const r = uninstall(['--force']);
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toContain(`${repo}/.gstack/`);
|
||||
expect(r.stdout).toContain(`${repo}/.gstack-worktrees/`);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack'))).toBe(false);
|
||||
expect(fs.existsSync(path.join(repo, '.gstack-worktrees'))).toBe(false);
|
||||
expect(fs.existsSync(path.join(home, '.gstack'))).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user