fix: pre-landing review fixes, pass 3

bin/gstack-decision-trail: hasHeader looks for a header line anywhere in
the file, so a blank or hand-written trail gets exactly one header rather
than one per append. bin/gstack-worktree-audit: the broken-worktree check
uses rev-parse --show-cdup and the presence of /.git instead of
comparing two spellings of the same path, which differ on Git for Windows.
Test: the blank-trail case appends twice and asserts a single header.

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 22:06:53 +00:00
1 parent c555ce3a83
commit 96927f2d4c
3 files changed
+9 -5

No files matched your search

+4 -4
View File
@@ -82,8 +82,8 @@ 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.
// A file whose first line is not the header (empty, blank, or rows written by
// hand) gets one appended; readRows skips a header line wherever it sits.
// A file with no header line anywhere (empty, blank, or rows written by hand)
// gets one appended once; readRows skips a header line wherever it sits.
const size = fs.existsSync(file) ? fs.statSync(file).size : 0;
if (size > 0 && !endsWithNewline(file, size)) fs.appendFileSync(file, "\n");
if (size === 0 || !hasHeader(file)) fs.appendFileSync(file, HEADER);
@@ -93,8 +93,8 @@ function appendRow(file: string, cells: readonly string[]): number {
}
function hasHeader(file: string): boolean {
const first = fs.readFileSync(file, "utf8").split("\n", 1)[0] ?? "";
return first.replace(/^\uFEFF/, "").replace(/\r$/, "") === HEADER.trimEnd();
return fs.readFileSync(file, "utf8").replace(/^\uFEFF/, "").split("\n")
.some(line => line.replace(/\r$/, "") === HEADER.trimEnd());
}
function endsWithNewline(file: string, size: number): boolean {
+3 -1
View File
@@ -137,7 +137,9 @@ git worktree list --porcelain | awk 'BEGIN { RS=""; FS="\n" } {
# git -C read describe whichever repository contains the directory, so its
# own files would be invisible: report it broken, never safe.
case "$flags" in *P*) printf '%s\t?\t?\tbroken\t?\t-\t-\treview\t%s\n' "$(du -sh "$wt" 2>/dev/null | awk '{print $1}')" "$wt"; continue ;; esac
if [ "$(git -C "$wt" rev-parse --show-toplevel 2>/dev/null)" != "$(cd "$wt" && pwd -P)" ]; then
# --show-cdup is empty only when the directory is itself a toplevel; a path
# comparison would misfire where git and the shell spell one path differently.
if [ ! -e "$wt/.git" ] || [ -n "$(git -C "$wt" rev-parse --show-cdup 2>/dev/null || echo broken)" ]; then
printf '%s\t?\t?\tbroken\t?\t-\t-\treview\t%s\n' "$(du -sh "$wt" 2>/dev/null | awk '{print $1}')" "$wt"
continue
fi
+2
View File
@@ -79,9 +79,11 @@ describe("gstack-decision-trail append", () => {
const blank = path.join(tmp, "blank.tsv");
fs.writeFileSync(blank, "\n");
expect(run(blank, "p", "d", "w", "e", "r").status).toBe(0);
expect(run(blank, "p2", "d", "w", "e", "r").status).toBe(0);
const lines = fs.readFileSync(blank, "utf8").split("\n");
expect(lines[1]).toBe("ts\tphase\tdecision\twhy\tevidence\tresult");
expect(lines[2]!.split("\t").slice(1)).toEqual(["p", "d", "w", "e", "r"]);
expect(lines.filter(l => l === "ts\tphase\tdecision\twhy\tevidence\tresult")).toHaveLength(1);
const bom = path.join(tmp, "bom.tsv");
fs.writeFileSync(bom, "\uFEFFts\tphase\tdecision\twhy\tevidence\tresult\n");
expect(run("--run", "r1", bom, "p", "d", "w", "e", "r").status).toBe(0);