CalvinBackup/gstack
1
0 Fork 0
mirror of https://github.com/garrytan/gstack.git synced 2026-11-11 03:00:26 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix: pre-landing review of the pstack ports

Browse Source 
bin/gstack-worktree-audit: a worktree path with a space is read whole (sed
on the porcelain 'worktree ' line, not awk's second field); a registered
worktree whose directory is gone prints a 'prunable' row instead of being
skipped; 'safe' now requires a clean tree and either merged into the base
or a closed/merged PR with every commit pushed, so unpushed commits or
untracked files on a merged branch read 'review' (the port's rule called
them safe); fork PRs are dropped and an open PR wins over an older closed
one on a reused branch name; GIT_TERMINAL_PROMPT=0 on the base fetch.

bin/gstack-decision-trail: only a leading -h/--help is help, so a '-h'
cell is appended as data; a trail without a final newline, with CRLF rows
or with a repeated header line is appended to correctly; a run id is
limited to [A-Za-z0-9._:@/-] (exit 2) so a tab or quote cannot break the
start-row idempotence check.

Tests cover each case; the 'without gh' test resolves git from
'git --exec-path' instead of a PATH rebuilt from a tool list. Docs,
CHANGELOG and --help text describe the stricter buckets.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-26 21:49:56 +00:00
1 parent e012748959
commit c9ae990209
8 files changed
+161 -50
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

test/decision-trail.test.ts
+25
View File
@@ -58,6 +58,31 @@ describe("gstack-decision-trail append", () => {
expect(rows()[2]![1]).toBe("'+1");
});
test("a cell equal to -h or --help is data, a trail without a final newline or with CRLF rows is repaired on append, and a repeated header is never a row", () => {
const r = run(file, "phase", "--help", "why", "evidence", "-h");
expect(r.status).toBe(0);
expect(r.stdout).toBe(`appended row 1 to ${file}\n`);
expect(rows()[1]!.slice(1)).toEqual(["phase", "'--help", "why", "evidence", "'-h"]);
fs.appendFileSync(file, "x\ty\tz");
run(file, "p2", "d2", "w", "e", "r");
const all = rows();
expect(all[2]).toEqual(["x", "y", "z"]);
expect(all[3]!.slice(1)).toEqual(["p2", "d2", "w", "e", "r"]);
const crlf = path.join(tmp, "crlf.tsv");
fs.writeFileSync(crlf, "ts\tphase\tdecision\twhy\tevidence\tresult\r\n2026-01-01T00:00:00Z\ta\tb\tc\td\te\r\nts\tphase\tdecision\twhy\tevidence\tresult\r\n");
const start = run("start", crlf, "run-1");
expect(start.stdout).toBe(`appended start row 2 for run run-1 to ${crlf}\n`);
expect(fs.readFileSync(crlf, "utf8").split("\n").filter(Boolean).pop()!.split("\t")[3]).toBe("1 earlier row (2026-01-01T00:00:00Z..2026-01-01T00:00:00Z) written by other runs");
});
test("a run id with whitespace or quotes is a usage error on start and on --run", () => {
expect(run("start", file, "a\tb").status).toBe(2);
expect(run("start", file, "a b").status).toBe(2);
expect(run("--run", "x'y", file, "a", "b", "c", "d", "e").status).toBe(2);
expect(run("start", file, "jam_01ABC.turn-2:x/y@z").status).toBe(0);
expect(fs.readFileSync(file, "utf8").split("\n")).toHaveLength(3);
});
test("wrong arity, an unknown option and a bare call are usage errors; --help exits 0", () => {
expect(run(file, "only", "four", "cells", "here").status).toBe(2);
expect(run("--bogus", file, "a", "b", "c", "d", "e").status).toBe(2);
test/worktree-audit.test.ts
+47 -12
View File
@@ -94,7 +94,25 @@ beforeAll(() => {
fakeBin = path.join(tmp, "fakebin");
fs.mkdirSync(fakeBin);
// The fake gh prints the TSV the real one's --jq would.
fs.writeFileSync(path.join(fakeBin, "gh"), "#!/usr/bin/env bash\nprintf 'feat/wip\\t41\\topen\\nfeat/closed\\t40\\tclosed\\n'\n", { mode: 0o755 });
// spaced: a worktree whose path has a space, merged into main, clean → safe.
wts.spaced = path.join(tmp, "wt with space");
git(repo, "worktree", "add", "-q", "-b", "feat/spaced", wts.spaced);
commit(wts.spaced, "spaced.txt", "s\n", "spaced work");
git(repo, "merge", "-q", "--ff-only", "feat/spaced");
git(repo, "push", "-q", "origin", "main");
// fresh: head is in main but an untracked file exists only here → review, not safe.
wts.fresh = path.join(tmp, "wt-fresh");
git(repo, "worktree", "add", "-q", "-b", "feat/fresh", wts.fresh);
fs.writeFileSync(path.join(wts.fresh, "newfeature.ts"), "export const x = 1;\n");
// gone: registered, directory deleted → prunable.
wts.gone = path.join(tmp, "wt-gone");
git(repo, "worktree", "add", "-q", "-b", "feat/gone", wts.gone);
fs.rmSync(wts.gone, { recursive: true, force: true });
});
afterAll(() => {
if (repo) spawnSync("git", ["worktree", "prune"], { cwd: repo, timeout: 30_000 });
@@ -103,14 +121,19 @@ afterAll(() => {
describe("gstack-worktree-audit", () => {
test("classifies every worktree, reads the PR column through gh, the chat through CLAUDE_CONFIG_DIR, and changes nothing", () => {
const before = Object.values(wts).map(snapshot).join("\n") + snapshot(repo);
const live = Object.values(wts).filter(w => fs.existsSync(w));
const before = live.map(snapshot).join("\n") + snapshot(repo);
const r = audit();
expect(r.status).toBe(0);
expect(r.stderr).toContain("warn: no -b given; MERGED checks against origin/main");
expect(r.base).toBe("# base: origin/main");
expect(r.header).toEqual(["SIZE", "AGE", "MERGED", "DIRTY", "REMOTE", "PR", "LAST_CHAT", "BUCKET", "WORKTREE"]);
expect(r.rows).toHaveLength(5);
expect(r.rows).toHaveLength(8);
expect(r.byPath[repo]).toBeUndefined();
expect(r.byPath[wts.spaced]).toMatchObject({ MERGED: "YES", DIRTY: "clean", BUCKET: "safe" });
expect(r.byPath[wts.fresh]).toMatchObject({ MERGED: "YES", DIRTY: "scratch:1", REMOTE: "no-remote", BUCKET: "review" });
expect(r.byPath[wts.gone]).toMatchObject({ SIZE: "0", DIRTY: "missing", BUCKET: "prunable" });
expect(r.rows[r.rows.length - 1]!.WORKTREE).toBe(wts.gone);
const merged = r.byPath[wts.merged]!;
expect(merged).toMatchObject({ MERGED: "YES", DIRTY: "clean", REMOTE: "no-remote", PR: "-", LAST_CHAT: "-", BUCKET: "safe", AGE: "0d" });
@@ -123,30 +146,42 @@ describe("gstack-worktree-audit", () => {
expect(r.byPath[wts.stale]).toMatchObject({ MERGED: "no", DIRTY: "clean", REMOTE: "no-remote", PR: "-", LAST_CHAT: "-", BUCKET: "review" });
expect(r.byPath[wts.closed]).toMatchObject({ MERGED: "no", DIRTY: "clean", REMOTE: "pushed", PR: "#40/closed", BUCKET: "safe" });
const after = Object.values(wts).map(snapshot).join("\n") + snapshot(repo);
const after = live.map(snapshot).join("\n") + snapshot(repo);
expect(after).toBe(before);
});
test("an open PR holds a clean worktree, an unpushed commit reads ahead<n>, and -b picks the base", () => {
test("an unpushed commit on a branch with a closed PR is review, an open PR on the same name wins and holds it, and -b picks the base", () => {
git(wts.closed, "commit", "-q", "--allow-empty", "-m", "one more");
fs.writeFileSync(path.join(fakeBin, "gh"), "#!/usr/bin/env bash\nprintf 'feat/closed\\t40\\topen\\n'\n", { mode: 0o755 });
const r = audit("-b", "origin/main", "--no-fetch", repo);
expect(r.stderr).not.toContain("warn: no -b given");
expect(r.byPath[wts.closed]).toMatchObject({ REMOTE: "ahead1", PR: "#40/open", BUCKET: "hold-open-pr" });
const r1 = audit("-b", "origin/main", "--no-fetch", repo);
expect(r1.stderr).not.toContain("warn: no -b given");
expect(r1.byPath[wts.closed]).toMatchObject({ REMOTE: "ahead1", PR: "#40/closed", BUCKET: "review" });
fs.writeFileSync(path.join(fakeBin, "gh"), "#!/usr/bin/env bash\nprintf 'feat/closed\\t40\\tclosed\\nfeat/closed\\t42\\topen\\n'\n", { mode: 0o755 });
const r2 = audit("-b", "origin/main", "--no-fetch", repo);
expect(r2.byPath[wts.closed]).toMatchObject({ REMOTE: "ahead1", PR: "#42/open", BUCKET: "hold-open-pr" });
});
test("without gh the PR column is - with a warning, and a missing base ref or repo is an error", () => {
// Shadow every gh on PATH with a directory the audit cannot execute from: `command -v` skips a non-executable file.
// A PATH with every tool the audit uses except gh. git goes in as the real binary from its
// exec path, since a distribution's /usr/bin/git may be a wrapper script that looks git up on PATH.
const noGh = path.join(tmp, "nogh");
fs.mkdirSync(noGh);
for (const tool of ["git", "bash", "du", "awk", "sed", "sort", "head", "find", "xargs", "stat", "date", "mktemp", "grep", "rm"]) {
const found = (process.env.PATH ?? "").split(path.delimiter).map(d => path.join(d, tool)).find(p => fs.existsSync(p));
if (found) fs.symlinkSync(found, path.join(noGh, tool));
const onPath = (tool: string) => (process.env.PATH ?? "").split(path.delimiter).map(d => path.join(d, tool)).find(p => fs.existsSync(p));
const execPath = spawnSync("git", ["--exec-path"], { encoding: "utf8", timeout: 30_000 }).stdout.trim();
const realGit = [path.join(execPath, "git"), onPath("git")].find(p => p && fs.existsSync(p))!;
fs.symlinkSync(fs.realpathSync(realGit), path.join(noGh, "git"));
for (const tool of ["bash", "sh", "env", "du", "awk", "sed", "sort", "head", "find", "xargs", "stat", "date", "mktemp", "grep", "rm", "cat", "dirname", "basename", "uname", "tr", "cut", "wc"]) {
const found = onPath(tool);
if (found) fs.symlinkSync(fs.realpathSync(found), path.join(noGh, tool));
}
const r = spawnSync("bash", [BIN, "--no-fetch"], { cwd: repo, encoding: "utf8", timeout: 60_000, env: { ...process.env, PATH: noGh, CLAUDE_CONFIG_DIR: claudeDir, HOME: tmp } });
expect(r.status).toBe(0);
expect(r.stderr).toContain("PR column is -");
expect(r.stdout.split("\n").filter(Boolean).slice(2).every(l => l.split("\t")[5] === "-")).toBe(true);
fs.writeFileSync(path.join(fakeBin, "gh"), "#!/usr/bin/env bash\necho 'gh: not logged in' >&2; exit 4\n", { mode: 0o755 });
const failing = audit("--no-fetch");
expect(failing.status).toBe(0);
expect(failing.stderr).toContain("gh could not list this repository's PRs; PR column is -");
expect(failing.rows.every(row => row.PR === "-")).toBe(true);
expect(audit("-b", "origin/nope", "--no-fetch").status).toBe(1);
expect(audit("--no-fetch", path.join(tmp, "not-a-repo")).status).toBe(1);
expect(audit("--bogus").status).toBe(2);