From c9ae99020945b79c29b159cd3bdb7e6ed60b4bfa Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 21:49:56 +0000 Subject: [PATCH] fix: pre-landing review of the pstack ports 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> --- CHANGELOG.md | 8 ++--- bin/gstack-decision-trail | 35 +++++++++++++++++---- bin/gstack-worktree-audit | 56 ++++++++++++++++++++++----------- docs/CONTRIBUTOR_REFERENCE.md | 19 ++++++----- docs/standalone-binaries.md | 2 +- docs/troubleshooting.md | 7 +++-- test/decision-trail.test.ts | 25 +++++++++++++++ test/worktree-audit.test.ts | 59 ++++++++++++++++++++++++++++------- 8 files changed, 161 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a36323aaf..efa676e09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,12 +5,12 @@ **Two read-and-write helpers absorbed from pstack: a decision trail a reviewer reads without the transcript, and a worktree audit that tells you which Conductor worktrees are safe to remove.** Both are ports of Lauren Tan's MIT scripts (header credit in each file, `licenses/MIT-pstack.txt`, NOTICE.md), approved with the rest of the pstack absorb (Garry, 2026-10-26: "Do it, send"). ### Added -- **`bin/gstack-decision-trail `**: appends one UTC-stamped row to a run's TSV decision trail, writing the header once and only ever appending; cells are flattened to one line and a cell that starts with `=`, `+`, `-` or `@` gets a leading quote so evidence lifted from a PR title or a filename cannot run as a formula in a reviewer's spreadsheet. `start ` appends a run's `start` row (naming the earlier rows and `run:`) and is a no-op while that row is the newest start row, so a rerun writes nothing twice; `--run ` on an append refuses with `DECISION_TRAIL_START_REQUIRED` (exit 3) until the run has opened its stretch. The end-of-run rule, in `docs/unattended.md` ("The decision trail") and `autoplan/DRIVER.md`: audit the trail against the transcript and correct the log, not the story (a wrong row is superseded, never edited), then a reviewer on another model family reads it and the report ends with an `Attention` section (`reviewed by `, the flags, or `No flags`). -- **`bin/gstack-worktree-audit [-b ] [--no-fetch] [repo]`** (bash, read-only): one TSV row per git worktree beside the main one, largest first: size, age, merged into the base, `clean`/`wip:`/`scratch:`, `pushed`/`ahead`/`no-remote`/`detached`, the PR on its branch through `gh`'s own `--jq` (no `jq` needed), the newest Claude Code chat that ran in the worktree (`${CLAUDE_CONFIG_DIR:-~/.claude}/projects/`) and a bucket: `hold-wip`, `hold-open-pr`, `verify-recent-chat`, `safe`, `review`. It never deletes or prunes. `/landing-report` offers it when the sibling table shows stale worktrees; `docs/CONTRIBUTOR_REFERENCE.md` has the guide for Conductor users, `docs/standalone-binaries.md` lists both bins. +- **`bin/gstack-decision-trail `**: appends one UTC-stamped row to a run's TSV decision trail, writing the header once and only ever appending; cells are flattened to one line and a cell that starts with `=`, `+`, `-` or `@` gets a leading quote so evidence lifted from a PR title or a filename cannot run as a formula in a reviewer's spreadsheet. `start ` appends a run's `start` row (naming the earlier rows and `run:`; the id is letters, digits and `. _ : @ / -`, so a tab or quote in it is a usage error rather than a row that breaks the idempotence check) and is a no-op while that row is the newest start row, so a rerun writes nothing twice; only a leading `-h`/`--help` is help, so a cell whose text is `-h` is appended as data; a trail without a final newline, with CRLF rows or with a repeated header line is appended to correctly; `--run ` on an append refuses with `DECISION_TRAIL_START_REQUIRED` (exit 3) until the run has opened its stretch. The end-of-run rule, in `docs/unattended.md` ("The decision trail") and `autoplan/DRIVER.md`: audit the trail against the transcript and correct the log, not the story (a wrong row is superseded, never edited), then a reviewer on another model family reads it and the report ends with an `Attention` section (`reviewed by `, the flags, or `No flags`). +- **`bin/gstack-worktree-audit [-b ] [--no-fetch] [repo]`** (bash, read-only): one TSV row per git worktree beside the main one, largest first: size, age, merged into the base, `clean`/`wip:`/`scratch:`, `pushed`/`ahead`/`no-remote`/`detached`, the PR on its branch through `gh`'s own `--jq` (no `jq` needed), the newest Claude Code chat that ran in the worktree (`${CLAUDE_CONFIG_DIR:-~/.claude}/projects/`) and a bucket: `prunable` (registered, directory gone), `hold-wip`, `hold-open-pr`, `verify-recent-chat`, `safe`, `review`. `safe` means nothing exists only in that worktree: a clean tree that is merged into the base or whose PR is closed or merged with every commit pushed; an unpushed commit or an untracked file on a merged branch reads `review`. A fork's PR on a same-named branch is ignored and an open PR wins over an older closed one on a reused name. Worktree paths with spaces are read whole. It never deletes or prunes. `/landing-report` offers it when the sibling table shows stale worktrees; `docs/CONTRIBUTOR_REFERENCE.md` has the guide for Conductor users, `docs/standalone-binaries.md` lists both bins. ### Tests -- `test/decision-trail.test.ts`: header once (an empty file gets one, a second append does not), the formula guard on every leading byte it covers, flattened tabs and line breaks, the start row's text and its idempotent rerun, a second run's start row, the `--run` refusal and admission, usage errors. -- `test/worktree-audit.test.ts`: a bare origin, a clone and five worktrees (merged, wip with an open PR, scratch with a chat today, stale, closed PR) classified into every bucket through a fake `gh` and a `CLAUDE_CONFIG_DIR` transcript directory; the fixture is byte-identical after the audit; `-b`, `--no-fetch`, `ahead`, no `gh` on PATH, a missing base and a non-repo path. +- `test/decision-trail.test.ts`: header once (an empty file gets one, a second append does not), the formula guard on every leading byte it covers, flattened tabs and line breaks, the start row's text and its idempotent rerun, a second run's start row, the `--run` refusal and admission, a `-h` cell appended as data, a missing final newline, CRLF rows and a repeated header, run ids with whitespace or quotes refused, usage errors. +- `test/worktree-audit.test.ts`: a bare origin, a clone and eight worktrees (merged, wip with an open PR, scratch with a chat today, stale, closed PR, a merged one whose path has a space, a merged one with only an untracked file, a registered one whose directory is gone) classified into every bucket through a fake `gh` and a `CLAUDE_CONFIG_DIR` transcript directory; the fixture is byte-identical after the audit; `-b`, `--no-fetch`, an unpushed commit on a closed PR reading `review` and an open PR on the reused name winning, no `gh` on PATH (git resolved from `git --exec-path`, so a distribution's wrapper script does not matter), a failing `gh`, a missing base and a non-repo path. ### Gate integrity - No threshold, timeout, trial count or case kind changed. No pinned skill changed; `landing-report/SKILL.md` grows by one line and is unpinned. `test/autoplan-driver.test.ts`'s line cap on `autoplan/DRIVER.md` moves 160 → 162 (measured 162) for the decision-trail paragraph at the end of the coordinator contract section; the file was at the cap before. diff --git a/bin/gstack-decision-trail b/bin/gstack-decision-trail index 04f20b4e6..a711051e3 100755 --- a/bin/gstack-decision-trail +++ b/bin/gstack-decision-trail @@ -49,7 +49,8 @@ spreadsheet cannot run it as a formula. Nothing is ever rewritten or removed. start appends this run's start row (phase 'start', evidence 'run:') naming the rows earlier runs wrote. Running it again for the -same run is a no-op while that start row is still the newest one. +same run is a no-op while that start row is still the newest one. A run id is +letters, digits and . _ : @ / - (a Capy jam_ id, a session id, a PR number). --run on an append refuses with DECISION_TRAIL_START_REQUIRED (exit 3) when the trail has rows and the newest start row is another run's. @@ -66,8 +67,10 @@ function readRows(file: string): Row[] { if (!fs.existsSync(file)) return []; const lines = fs.readFileSync(file, "utf8").split("\n"); const rows: Row[] = []; - lines.forEach((text, i) => { - if (text === "" || (i === 0 && text === HEADER.trimEnd())) return; + lines.forEach((raw, i) => { + const text = raw.endsWith("\r") ? raw.slice(0, -1) : raw; + // A repeated header (the append-on-flaky-mount case) is never a row. + if (text === "" || text === HEADER.trimEnd()) return; rows.push({ line: i + 1, cells: text.split("\t") }); }); return rows; @@ -78,13 +81,30 @@ function appendRow(file: string, cells: readonly string[]): number { if (dir && dir !== "." && !fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); // Append the header too, never truncate: on a flaky mount the worst case is // one stray header line, not lost rows. - const needsHeader = !fs.existsSync(file) || fs.statSync(file).size === 0; - if (needsHeader) fs.appendFileSync(file, HEADER); + const size = fs.existsSync(file) ? fs.statSync(file).size : 0; + if (size === 0) fs.appendFileSync(file, HEADER); + else if (!endsWithNewline(file, size)) fs.appendFileSync(file, "\n"); const ts = new Date().toISOString().replace(/\.\d{3}Z$/, "Z"); fs.appendFileSync(file, [ts, ...cells.map(clean)].join("\t") + "\n"); return readRows(file).length; } +function endsWithNewline(file: string, size: number): boolean { + const fd = fs.openSync(file, "r"); + try { + const last = Buffer.alloc(1); + fs.readSync(fd, last, 0, 1, size - 1); + return last[0] === 0x0a; + } finally { + fs.closeSync(fd); + } +} + +const RUN_ID = /^[A-Za-z0-9._:@/-]+$/; +function checkRunId(runId: string): void { + if (!RUN_ID.test(runId)) fail(`run id must match ${RUN_ID} (letters, digits, . _ : @ / -), got ${JSON.stringify(runId)}`, 2); +} + function newestStart(rows: Row[]): Row | undefined { for (let i = rows.length - 1; i >= 0; i--) if (rows[i]!.cells[1] === "start") return rows[i]; return undefined; @@ -96,7 +116,8 @@ function fail(message: string, code: number): never { } const argv = process.argv.slice(2); -if (argv.length === 0 || argv.includes("--help") || argv.includes("-h")) { +// Only a leading --help asks for help: a cell whose text is "-h" is data and gets appended (quoted). +if (argv.length === 0 || argv[0] === "--help" || argv[0] === "-h") { process.stdout.write(USAGE); process.exit(argv.length === 0 ? 2 : 0); } @@ -104,6 +125,7 @@ if (argv.length === 0 || argv.includes("--help") || argv.includes("-h")) { if (argv[0] === "start") { const [, file, runId, ...extra] = argv; if (!file || !runId || extra.length > 0) fail(`start takes \n${USAGE}`, 2); + checkRunId(runId); const rows = readRows(file); const newest = newestStart(rows); if (newest && newest.cells[4] === `run:${runId}`) { @@ -125,6 +147,7 @@ while (positional[0]?.startsWith("--")) { if (flag === "--run") { runId = positional.shift(); if (!runId) fail("--run needs an id", 2); + checkRunId(runId); } else fail(`unknown option ${flag}\n${USAGE}`, 2); } if (positional.length !== 6) fail(`expected and five cells, got ${positional.length} argument${positional.length === 1 ? "" : "s"}\n${USAGE}`, 2); diff --git a/bin/gstack-worktree-audit b/bin/gstack-worktree-audit index 4e28fc632..481b81bc1 100755 --- a/bin/gstack-worktree-audit +++ b/bin/gstack-worktree-audit @@ -3,14 +3,19 @@ # one: size, age of its last commit, whether its head is merged into the base, # tracked edits vs untracked scratch, remote state, the PR on its branch, the # newest Claude Code chat that ran in it, and a suggested bucket. One TSV row -# per worktree, largest first. It never deletes, prunes or checks anything out; -# removing a worktree stays a step a person takes after reading the table. +# per worktree, largest first. It never deletes, prunes, checks out or edits a +# worktree or a branch; its one write is the best-effort fetch of the base ref +# (skip it with --no-fetch). Removing a worktree stays a step a person takes +# after reading the table. # # Ported from pstack's poteto-mode/scripts/worktree-audit.sh (Copyright Lauren # Tan, MIT; https://github.com/cursor/plugins, pstack/skills/poteto-mode at # 99b609a7). Changes from the source: the PR column comes from gh alone through -# its own --jq (no jq binary), LAST_CHAT reads Claude Code's per-worktree -# transcript directory instead of Cursor's, and --no-fetch is new. License +# its own --jq (no jq binary) and ignores fork PRs, LAST_CHAT reads Claude +# Code's per-worktree transcript directory instead of Cursor's, worktree paths +# may contain spaces, a registered worktree whose directory is gone gets a +# `prunable` row, `safe` also requires a clean tree with nothing unpushed, and +# --no-fetch is new. License # text: licenses/MIT-pstack.txt; NOTICE.md carries the entry. # # Usage: gstack-worktree-audit [-b|--base ] [--no-fetch] [repo-path] @@ -24,14 +29,18 @@ # Columns: SIZE AGE MERGED DIRTY REMOTE PR LAST_CHAT BUCKET WORKTREE # MERGED YES when the worktree's head is an ancestor of the base. A # squash-merged branch is never an ancestor, so read PR with it. -# DIRTY clean | wip: (tracked edits) | scratch: (untracked only) +# DIRTY clean | wip: (tracked edits) | scratch: (untracked only) | +# missing (registered, directory gone: `git worktree prune` territory) # REMOTE pushed | ahead | no-remote | detached -# PR #/ from gh (open, merged, closed), or - without gh +# PR #/ from gh (open, merged, closed; an open PR wins when a +# branch name has several, fork PRs are ignored), or - without gh # LAST_CHAT date of the newest Claude Code transcript under # ${CLAUDE_CONFIG_DIR:-~/.claude}/projects//, or - -# BUCKET hold-wip (tracked edits) > hold-open-pr > verify-recent-chat (a -# chat in the last 4 days) > safe (merged, or its PR is closed or -# merged) > review (nothing says it is done) +# BUCKET prunable (directory gone) > hold-wip (tracked edits) > +# hold-open-pr > verify-recent-chat (a chat in the last 4 days) > +# safe (clean tree, and either merged into the base or its PR is +# closed or merged with every commit pushed) > review (anything +# else: unpushed commits, untracked files, nothing says it is done) set -u base="" @@ -52,7 +61,7 @@ cd "$repo" || exit 1 git rev-parse --git-dir >/dev/null 2>&1 || { echo "gstack-worktree-audit: not a git repository: $repo" >&2; exit 1; } # The main worktree is the first entry; every other one is a candidate. -main_wt=$(git worktree list --porcelain | awk '/^worktree /{print $2; exit}') +main_wt=$(git worktree list --porcelain | sed -n 's/^worktree //p' | head -1) if [ -z "$base" ]; then for cand in "$(git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null)" origin/main origin/master; do @@ -66,7 +75,7 @@ fi # explicit refspec creates the remote ref even in a single-branch clone. if [ "$fetch" = 1 ]; then case "$base" in - origin/*) git fetch origin "+refs/heads/${base#origin/}:refs/remotes/$base" --quiet 2>/dev/null \ + origin/*) GIT_TERMINAL_PROMPT=0 git fetch origin "+refs/heads/${base#origin/}:refs/remotes/$base" --quiet 2>/dev/null \ || echo "warn: could not fetch $base; MERGED may be stale" >&2 ;; esac fi @@ -81,12 +90,14 @@ else fi # PR state per branch, read once as "\t\t" lines through -# gh's own --jq. Empty when gh is missing or cannot read this repository. +# gh's own --jq; a fork's PR says nothing about a local branch of the same name, +# so cross-repository PRs are dropped. Empty when gh is missing or cannot read +# this repository. prs=$(mktemp) trap 'rm -f "$prs"' EXIT if command -v gh >/dev/null 2>&1; then - gh pr list --state all --limit 1000 --json number,state,headRefName \ - --jq '.[] | [.headRefName, (.number|tostring), (.state|ascii_downcase)] | @tsv' > "$prs" 2>/dev/null \ + gh pr list --state all --limit 1000 --json number,state,headRefName,isCrossRepository \ + --jq '.[] | select(.isCrossRepository | not) | [.headRefName, (.number|tostring), (.state|ascii_downcase)] | @tsv' > "$prs" 2>/dev/null \ || { : > "$prs"; echo "warn: gh could not list this repository's PRs; PR column is -" >&2; } else echo "warn: gh not on PATH; PR column is -" >&2 @@ -98,9 +109,12 @@ now=$(date +%s) echo "# base: $base" printf 'SIZE\tAGE\tMERGED\tDIRTY\tREMOTE\tPR\tLAST_CHAT\tBUCKET\tWORKTREE\n' -git worktree list --porcelain | awk '/^worktree /{print $2}' | while read -r wt; do +git worktree list --porcelain | sed -n 's/^worktree //p' | while read -r wt; do [ "$wt" = "$main_wt" ] && continue - [ -d "$wt" ] || continue + if [ ! -d "$wt" ]; then + printf '0\t?\t?\tmissing\t?\t-\t-\tprunable\t%s\n' "$wt" + continue + fi size=$(du -sh "$wt" 2>/dev/null | awk '{print $1}') head=$(git -C "$wt" rev-parse HEAD 2>/dev/null) @@ -124,8 +138,11 @@ git worktree list --porcelain | awk '/^worktree /{print $2}' | while read -r wt; || remote="ahead$(git -C "$wt" rev-list --count "origin/$branch..HEAD" 2>/dev/null)" else remote=no-remote; fi + # Several PRs can share a branch name over time; an open one describes the + # branch as it is now, so it wins over a closed or merged one. pr="-" - [ -n "$branch" ] && [ -s "$prs" ] && pr=$(awk -F'\t' -v b="$branch" '$1==b {print "#" $2 "/" $3; exit}' "$prs") + [ -n "$branch" ] && [ -s "$prs" ] && pr=$(awk -F'\t' -v b="$branch" \ + '$1==b { if ($3=="open" || $3=="draft") { print "#" $2 "/" $3; exit } if (first=="") first="#" $2 "/" $3 } END { if (first!="") print first }' "$prs" | head -1) [ -z "$pr" ] && pr="-" # Newest transcript in this worktree's own Claude Code project directory @@ -138,10 +155,13 @@ git worktree list --porcelain | awk '/^worktree /{print $2}' | while read -r wt; fi recent=$([ "$last_ts" -gt 0 ] 2>/dev/null && [ $(( (now - last_ts) / 86400 )) -le 4 ] && echo yes || echo no) + # safe means nothing in the worktree exists only there: a clean tree whose + # head is in the base, or whose PR is closed or merged with every commit + # pushed. Unpushed commits or untracked files on a merged branch are review. case "$dirty" in wip:*) bucket=hold-wip ;; *) case "$pr" in */open|*/draft) bucket=hold-open-pr ;; *) if [ "$recent" = yes ]; then bucket=verify-recent-chat - elif [ "$merged" = YES ] || [ "$pr" != "-" ]; then bucket=safe + elif [ "$dirty" = clean ] && { [ "$merged" = YES ] || { [ "$pr" != "-" ] && [ "$remote" = pushed ]; }; }; then bucket=safe else bucket=review; fi ;; esac ;; esac diff --git a/docs/CONTRIBUTOR_REFERENCE.md b/docs/CONTRIBUTOR_REFERENCE.md index b60620a05..4adbb59a6 100644 --- a/docs/CONTRIBUTOR_REFERENCE.md +++ b/docs/CONTRIBUTOR_REFERENCE.md @@ -182,10 +182,15 @@ beside the main one, largest first: size, age of the last commit, whether its head is merged into the base, `clean` / `wip:` (tracked edits) / `scratch:` (untracked only), `pushed` / `ahead` / `no-remote` / `detached`, the PR on its branch through `gh` (`#n/open|merged|closed`, or -`-`), the date of the newest Claude Code chat that ran in it, and a bucket: -`hold-wip`, then `hold-open-pr`, then `verify-recent-chat` (a chat in the last -four days), then `safe` (merged, or its PR is closed or merged), else -`review`. A squash-merged branch is never an ancestor of the base, so read -`MERGED` together with `PR`. The audit only reads; removing a worktree -(`git worktree remove `) stays your call after the table. `/landing-report` -points at it when the sibling table shows stale worktrees. +`-`; a fork's PR on a same-named branch is ignored, and an open PR wins over +an older closed one on a reused branch name), the date of the newest Claude +Code chat that ran in it, and a bucket: `prunable` (registered, directory +gone), then `hold-wip`, then `hold-open-pr`, then `verify-recent-chat` (a chat +in the last four days), then `safe`, else `review`. `safe` means nothing +exists only in that worktree: a clean tree whose head is merged into the base, +or whose PR is closed or merged with every commit pushed; an unpushed commit or +an untracked file on a merged branch is `review`. A squash-merged branch is +never an ancestor of the base, so read `MERGED` together with `PR`. The audit +only reads; removing a worktree (`git worktree remove `) stays your call +after the table. `/landing-report` points at it when the sibling table shows +stale worktrees. diff --git a/docs/standalone-binaries.md b/docs/standalone-binaries.md index c2db8030e..3d1df596c 100644 --- a/docs/standalone-binaries.md +++ b/docs/standalone-binaries.md @@ -13,7 +13,7 @@ Beyond the slash-command skills, gstack ships standalone CLIs for workflows that | `gstack-verify-gate` | **Verification stop hook (opt-in)** — blocks a Claude Code turn from ending until the project's declared verify command passes (after 3 blocked re-entries it yields with a loud still-RED warning instead of looping forever). Declare it on one line in CLAUDE.md: ``. Hooks bypass the permission system, so a declared command never runs until you trust it once per repo (`gstack-verify-gate --trust`); editing the command invalidates trust until re-granted, and every grant is audit-logged. `./setup` never registers it for you — opt in with `gstack-settings-hook add-event --event Stop --command ~/.claude/skills/gstack/bin/gstack-verify-gate --source verify-gate`, remove with `gstack-settings-hook remove-source --source verify-gate`. | | `gstack-memorable` | **Memorable recall bridge (opt-in, third party, Claude Code only)** — connects Claude Code to the external [Memorable](https://memorable.sh) CLI *through gstack* instead of the vendor's own installer, so the hook gets gstack's guarantees: an explicit consent key (`memorable_recall`, off by default, listed by `gstack-egress grants`), a fail-closed egress receipt for every prompt handed over (`gstack-egress list --sink memorable-recall`), a HIGH-tier secret pre-scan, a trust envelope and 8 KiB cap on whatever comes back, an allowlisted environment and process-group containment for the vendor process, and clean removal. `enable` registers the hook at the stable install with a 5 s timeout and never runs the vendor's own consent command; `disable` revokes the gate first and removes the entry by identity even after Claude Code strips the tag; `status` is read-only. gstack never installs Memorable, and what its binary sends is the vendor's claim, not gstack's. Not available on Windows yet. [Full guide](memorable-workflow-memory.md). | | `gstack-decision-trail` | **Decision trail** — append one row (ts, phase, decision, why, evidence, result) to a run's TSV trail so a reviewer reads what was chosen and why without the transcript. Header once, append-only, cells flattened to one line, and a cell starting with `=`, `+`, `-` or `@` quoted so a spreadsheet cannot run it. `start ` opens a run's stretch of a shared trail (idempotent), and `--run ` refuses an append until that start row is the newest. Ported from pstack's `log.sh` (MIT, NOTICE.md). [Rule](unattended.md#the-decision-trail). | -| `gstack-worktree-audit` | **Worktree audit** — read-only table of every git worktree beside the main one, largest first: size, age, merged into the base or not, `clean`/`wip:`/`scratch:`, `pushed`/`ahead`/`no-remote`/`detached`, the PR on its branch through `gh`, the newest Claude Code chat in it, and a bucket (`hold-wip`, `hold-open-pr`, `verify-recent-chat`, `safe`, `review`). `-b origin/` sets the base, `--no-fetch` skips the base fetch. Never deletes. Ported from pstack's `worktree-audit.sh` (MIT, NOTICE.md). [Guide](CONTRIBUTOR_REFERENCE.md#auditing-conductor-worktrees). | +| `gstack-worktree-audit` | **Worktree audit** — read-only table of every git worktree beside the main one, largest first: size, age, merged into the base or not, `clean`/`wip:`/`scratch:`, `pushed`/`ahead`/`no-remote`/`detached`, the PR on its branch through `gh` (fork PRs ignored), the newest Claude Code chat in it, and a bucket (`prunable` for a registered worktree whose directory is gone, `hold-wip`, `hold-open-pr`, `verify-recent-chat`, `safe` only for a clean tree with nothing unpushed, else `review`). `-b origin/` sets the base, `--no-fetch` skips the base fetch. Never deletes. Ported from pstack's `worktree-audit.sh` (MIT, NOTICE.md). [Guide](CONTRIBUTOR_REFERENCE.md#auditing-conductor-worktrees). | | `gstack-wtree` | **Working-tree fingerprint** — prints a content hash of what's actually on disk (temp index seeded from the stat cache, ~40x cheaper than a full re-hash; untracked source counts, gitignored scratch doesn't). Identical content fingerprints identically through commits, rebases, amends, and squashes — it's what binds reviews and test evidence to content instead of commit SHAs. | | `gstack-review-log` | **Review-pass receipts** — `--start ` captures the working-tree fingerprint before a diff review; `'' --finish ` consumes that single-use, repository/branch/skill-scoped receipt. Binding requires matching start/end content and reviewer-reported `completed:true` and `converged:true`; it is not independent proof that a model read the source. `--check-shared-libs ` reads a current finding as JSON on stdin and checks prior Skip decisions against the actual branch, capture and eligible raw source blobs. It returns `reusable:false` when proof is missing or unsafe, including older records without logger-versioned coverage. Final review logging computes shared-code fingerprints and coverage rather than trusting supplied proof. | | `gstack-review-read` | **Review freshness** — emits review records with computed `review_freshness.status` and `reason`: CURRENT, STALE, or UNVERIFIED for diff reviews. `/ship` and `/land-and-deploy` use the same grade; a matching commit alone never certifies a diff review. [Dashboard rules](skills.md#review-readiness-dashboard). | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index da874bf6f..5fb2739d3 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4552,5 +4552,8 @@ Without `-b` the audit compares each worktree's head against the remote's default branch (then `origin/main`, then `origin/master`), which is not always the branch PRs merge into; pass `-b origin/`. A squash-merged branch is never an ancestor of the base, so `MERGED no` beside `PR #n/merged` -is normal and the bucket is still `safe`. The audit only reads; a worktree it -calls `safe` is still yours to remove. +is normal and the bucket is still `safe` while the tree is clean and every +commit is pushed; an unpushed commit or an untracked file on that branch makes +it `review`, because deleting the worktree would lose it. The audit only reads; +a worktree it calls `safe` is still yours to remove, and a `prunable` row names +a registered worktree whose directory is already gone (`git worktree prune`). diff --git a/test/decision-trail.test.ts b/test/decision-trail.test.ts index d4034fcac..eb994e9ad 100644 --- a/test/decision-trail.test.ts +++ b/test/decision-trail.test.ts @@ -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); diff --git a/test/worktree-audit.test.ts b/test/worktree-audit.test.ts index a75ce2af1..73780465f 100644 --- a/test/worktree-audit.test.ts +++ b/test/worktree-audit.test.ts @@ -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, 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);