refactor(cli): keep the gitpick pin in one constant and test reinit and sync

.agents/skills/runtime-apis/index.md

@@ -35,6 +35,8 @@ runs under the system Node while Docker and Vercel serve it under Bun.
 | `tests/github/scripts/ensure-remote-branches.test.ts` | Bun | `node:fs` (mkdtempSync, rmSync); `node:os` (tmpdir); `node:path` (join) |
 | `tests/github/workflows/auto-labeler.test.ts` | Bun | `node:module` (createRequire); `node:path` (join) |
 | `tests/packages/cli/bin/commands/init.test.ts` | Bun | `node:fs` (mkdirSync, mkdtempSync, rmSync, writeFileSync); `node:os` (tmpdir); `node:path` (join) |
+| `tests/packages/cli/bin/commands/reinit.test.ts` | Bun | `node:child_process` (execFileSync); `node:fs` (mkdtempSync, readFileSync, rmSync, writeFileSync); `node:os` (tmpdir); `node:path` (join); `node:util` (stripVTControlCharacters) |
+| `tests/packages/cli/bin/commands/sync.test.ts` | Bun | `node:child_process` (execFileSync); `node:fs` (mkdtempSync, readFileSync, rmSync, writeFileSync); `node:os` (tmpdir); `node:path` (join); `node:util` (stripVTControlCharacters) |
 | `tests/packages/cli/features-consistency.test.ts` | Bun | `node:fs` (readFileSync); `node:path` (join) |
 | `tests/packages/cli/src/convert.test.ts` | Bun | `node:fs` (mkdtempSync, readFileSync, rmSync); `node:os` (tmpdir); `node:path` (basename, join) |
 | `tests/packages/cli/src/db.test.ts` | Bun | `node:fs` (mkdtempSync, readFileSync, rmSync, writeFileSync); `node:os` (tmpdir); `node:path` (join) |

.github/notes/plans/README.md

@@ -31,7 +31,7 @@ This is the internal, fork-excluded backlog. It is separate from the published `
 - [Derive BlogPostMeta from the blog zod schema](blog-meta-from-schema.md) - carved out of content-source-consolidation; a decouple-vs-derive tradeoff, not a mechanical rename (PR #691 review).
 - [Security headers and a durable rate-limit store](hardening-refactors.md) - what is left of the external evaluation's hardening list: its three named items shipped (CI gates, `serverSecret`, the agent sign-in toggle); default security headers and CSP, a rate-limit store that survives a restart, and the env-shape tests did not.
 - [WebSockets on Vercel through Bun.serve()](vercel-bun-serve-websockets.md) - the Node adapter split exists because Vercel could not run `Bun.serve()`; its Bun runtime now can, in beta and with documented differences, which would let one code path serve every host.
-- [CLI developer experience](cli-dx.md) - what the 2026-07-04 CLI audit left open: `--dry-run` on `reinit` and `sync`, a `--ref` with a provenance stamp, a confirm on `sync`, an update notice, tests for the two untested commands, `--verbose`, and one constant for the gitpick pin.
+- [CLI developer experience](cli-dx.md) - what the 2026-07-04 CLI audit left open: `--dry-run` on `reinit` and `sync`, a `--ref` with a provenance stamp, a confirm on `sync`, an update notice, and `--verbose`.
 - [Activity log retention and indexes](console-activity-log.md) - the log shipped in PR #762 with no retention and no indexes, both deliberately; the table grows without bound, and `created_at` is the first index worth adding once the list feels slow.
 
 ### Architecture deepenings (2026-07-12 review, deep-module lens)

.github/notes/plans/cli-dx.md

@@ -3,14 +3,12 @@
 - Status: backlog
 - Links: the CLI DX audit of 2026-07-04 (deleted once folded in here; ten findings, ranked), PR #645 (findings 1 and 6)
 
-The audit covered `packages/cli` (`init`, `reinit`, `sync`) for two readers: someone running the CLI, and someone maintaining it. Three findings are closed: subprocess failures now print their real output (1), a mistyped flag prints that command's help (6), and the npm page has a README, since npm packs `README.md` whatever `files` says (8). What is left, in the audit's own order of payoff:
+The audit covered `packages/cli` (`init`, `reinit`, `sync`) for two readers: someone running the CLI, and someone maintaining it. Five findings are closed: subprocess failures now print their real output (1), a mistyped flag prints that command's help (6), `reinit` and `sync` have tests for their repo guards and for the branching they print (7), the npm page has a README, since npm packs `README.md` whatever `files` says (8), and the gitpick pin is one constant in `src/git.ts`, as `pglaunch` is in `src/db.ts` (10). What is left, in the audit's own order of payoff:
 
 - **`--dry-run` on `reinit` and `sync`** (finding 2, high). Only `init` previews its plan. `reinit` deletes every tracked file and `sync` overwrites starter files in place, and neither can show what it would delete, keep or overwrite first. `reinit` should list kept (`.git`, `.env*`) against deleted; `sync` should list its preserve set and the merge plan without touching the tree.
 - **`--ref` and a provenance stamp** (finding 3, high, structural). All three commands fetch `main`, so a scaffold cannot be pinned or reproduced, and a fork records nothing about the starter commit it came from. Accept `--ref <branch|tag|sha>` on all three, threaded through the fetch and overlay helpers, and write a stamp on `init` and `sync` (ref, resolved sha, CLI version, date). That is what would let `sync` say how far behind a fork is.
 - **`sync` confirms nothing and takes no `--yes`** (finding 4, medium). `reinit` confirms and honours `-y`; `sync` just runs, though it overwrites tracked files and runs an install. Rollback makes it safer, but the asymmetry surprises, and its `--help` does not explain the `[dir]` positional. Add the confirm and the flag, or record that skipping it is deliberate.
 - **No update notice** (finding 5, medium). An old cached `npx zerostarter` runs silently. A best-effort comparison of the running version with the npm `latest` dist-tag, short timeout and never blocking, would print one line on a mismatch.
-- **Command flows are only partly tested** (finding 7, medium). `init` and the prompt layer now have tests under `tests/packages/cli/bin/commands/`; `reinit` and `sync` have none. Extract their pure branching into units and cover those; network, git and docker stay out of scope.
 - **No `--verbose`** (finding 9, low). Nothing surfaces the raw spawned command and its full output when a step misbehaves. Complements the error-legibility work.
-- **`gitpick@6.0.0` is a literal at two call sites** (finding 10, low). `pglaunch` is already one constant in `src/db.ts`; `src/git.ts` still repeats the gitpick pin, so a bump touches two places. Pinning is the right policy; hoist it to one constant.
 
 The audit's suggested order still holds: dry-run parity first, since it is the trust gap on the two destructive commands, then the ref and the stamp, then the polish.

packages/cli/bin/commands/reinit.ts

@@ -34,6 +34,14 @@ Options:
   -y, --yes      Skip the confirmation prompt
   -h, --help     Display help`
 
+// What to run after a re-scaffold; the database URL step only while .env has none.
+export const nextSteps = (hasUrl: boolean): string[] => [
+  ...(hasUrl ? [] : ["set POSTGRES_URL in .env"]),
+  orange("bun run db:migrate"),
+  orange("bun run dev"),
+  orange("git push"),
+]
+
 export const reinit = async (argv: string[]) => {
   const { positionals, values } = parseArgsOrExit(helpMessage, {
     allowPositionals: true,
@@ -95,11 +103,6 @@ export const reinit = async (argv: string[]) => {
     },
   )
 
-  const steps: string[] = []
-  if (!hasPostgresUrl(target)) steps.push("set POSTGRES_URL in .env")
-  steps.push(orange("bun run db:migrate"))
-  steps.push(orange("bun run dev"))
-  steps.push(orange("git push"))
-  note(steps.join("\n"), "Next steps")
+  note(nextSteps(hasPostgresUrl(target)).join("\n"), "Next steps")
   outro(green(`${name} re-scaffolded; .git history, remote, and .env* files are intact`))
 }

packages/cli/bin/commands/sync.ts

@@ -39,6 +39,50 @@ you commit yourself.
 Options:
   -h, --help     Display help`
 
+// Name what the reconcile left alone or took on a guess, so the sync lands as a reviewable diff rather than a silent overwrite.
+export const reportReconcile = (skills: SkillReconcile, guide: GuideReconcile): void => {
+  const skillFiles = (names: string[]) => names.map((name) => `.agents/skills/${name}/SKILL.md`)
+  const plural = (names: string[]) => (names.length === 1 ? "" : "s")
+
+  if (skills.forkOwned.length > 0) {
+    logStep(
+      `Left ${skills.forkOwned.length} skill${plural(skills.forkOwned)} you own untouched:`,
+      skillFiles(skills.forkOwned),
+    )
+  }
+
+  if (skills.customized.length > 0) {
+    logWarn(
+      `Kept your edits to ${skills.customized.length} skill${plural(skills.customized)}, so they did not take the update:`,
+      skillFiles(skills.customized),
+    )
+    logStep("To take upstream's version instead, delete the skill directory and sync again.")
+  }
+
+  if (guide === "customized") {
+    logStep(
+      "Kept your AGENTS.md, which you have edited, so it did not take the update. To take the starter's version instead, delete it, commit that on its own, and sync again.",
+    )
+  }
+  // A guide with no sync record that is not the stub an older CLI wrote is the fork's own work, so it is never replaced on a guess.
+  if (guide === "forkOwned") {
+    logStep(
+      "Kept your AGENTS.md. The starter now ships its full agent guide; to take it, delete AGENTS.md, commit that on its own, and sync again.",
+    )
+  }
+
+  // A fork synced before the CLI started recording what it wrote has nothing to compare against, so these took the update on a guess. Naming them is the difference between a reviewable diff and a silent loss.
+  if (skills.unverified.length > 0) {
+    logWarn(
+      `Updated ${skills.unverified.length} skill${plural(skills.unverified)} with no sync record, so any edits of yours are in the diff rather than the file:`,
+      skillFiles(skills.unverified),
+    )
+    logStep(
+      "Restore any with: git restore --source=HEAD -- .agents/skills/<name>/SKILL.md. Later syncs track them.",
+    )
+  }
+}
+
 // Re-baseline a fork on the latest ZeroStarter, preserving its content, branding, and package.json.
 export const sync = async (argv: string[]) => {
   const { positionals, values } = parseArgsOrExit(helpMessage, {
@@ -128,46 +172,7 @@ export const sync = async (argv: string[]) => {
     )
   }
 
-  const skillFiles = (names: string[]) => names.map((name) => `.agents/skills/${name}/SKILL.md`)
-  const plural = (names: string[]) => (names.length === 1 ? "" : "s")
-
-  if (skills.forkOwned.length > 0) {
-    logStep(
-      `Left ${skills.forkOwned.length} skill${plural(skills.forkOwned)} you own untouched:`,
-      skillFiles(skills.forkOwned),
-    )
-  }
-
-  if (skills.customized.length > 0) {
-    logWarn(
-      `Kept your edits to ${skills.customized.length} skill${plural(skills.customized)}, so they did not take the update:`,
-      skillFiles(skills.customized),
-    )
-    logStep("To take upstream's version instead, delete the skill directory and sync again.")
-  }
-
-  if (guide === "customized") {
-    logStep(
-      "Kept your AGENTS.md, which you have edited, so it did not take the update. To take the starter's version instead, delete it, commit that on its own, and sync again.",
-    )
-  }
-  // A guide with no sync record that is not the stub an older CLI wrote is the fork's own work, so it is never replaced on a guess.
-  if (guide === "forkOwned") {
-    logStep(
-      "Kept your AGENTS.md. The starter now ships its full agent guide; to take it, delete AGENTS.md, commit that on its own, and sync again.",
-    )
-  }
-
-  // A fork synced before the CLI started recording what it wrote has nothing to compare against, so these took the update on a guess. Naming them is the difference between a reviewable diff and a silent loss.
-  if (skills.unverified.length > 0) {
-    logWarn(
-      `Updated ${skills.unverified.length} skill${plural(skills.unverified)} with no sync record, so any edits of yours are in the diff rather than the file:`,
-      skillFiles(skills.unverified),
-    )
-    logStep(
-      "Restore any with: git restore --source=HEAD -- .agents/skills/<name>/SKILL.md. Later syncs track them.",
-    )
-  }
+  reportReconcile(skills, guide)
 
   note(
     [

packages/cli/src/git.ts

@@ -3,6 +3,8 @@ import { join } from "node:path"
 import { exists } from "@/io"
 import { ok, run, runTail } from "@/spawn"
 
+const GITPICK = "gitpick@6.0.0"
+
 // Install dependencies in `dir`, showing a rolling window of bun's output that collapses to a done step, keeping the tail (which carries bun's "N packages installed [time]" summary line). Runs the fork's lifecycle scripts (git hooks via prepare, catalog sync) as a normal `bun install` would.
 export const bunInstall = async (dir: string): Promise<void> => {
   await runTail("bun", ["install"], {
@@ -35,19 +37,14 @@ export const bunAvailable = async (
 
 // Fetch the latest zerostarter scaffold into `dir` (a gitpick subtree overlay, no .git history). --bun runs gitpick under the Bun runtime, not Node.
 export const fetchZerostarter = async (dir: string, ref = "main"): Promise<void> => {
-  await run("bunx", [
-    "--bun",
-    "gitpick@6.0.0",
-    `https://github.com/nrjdalal/zerostarter/tree/${ref}`,
-    dir,
-  ])
+  await run("bunx", ["--bun", GITPICK, `https://github.com/nrjdalal/zerostarter/tree/${ref}`, dir])
 }
 
 // Overlay the latest zerostarter onto a fork (gitpick -o); .gitpickignore paths and fork-added files are kept.
 export const overlayZerostarter = async (dir: string, ref = "main"): Promise<void> => {
   await run("bunx", [
     "--bun",
-    "gitpick@6.0.0",
+    GITPICK,
     `https://github.com/nrjdalal/zerostarter/tree/${ref}`,
     dir,
     "-o",

tests/packages/cli/bin/commands/reinit.test.ts

@@ -0,0 +1,105 @@
+import { afterEach, beforeEach, describe, expect, test } from "bun:test"
+import { execFileSync } from "node:child_process"
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"
+import { tmpdir } from "node:os"
+import { join } from "node:path"
+import { stripVTControlCharacters } from "node:util"
+
+import { nextSteps, reinit } from "../../../../../packages/cli/bin/commands/reinit"
+
+let dir: string
+const git = (...args: string[]) => execFileSync("git", args, { cwd: dir, encoding: "utf8" })
+
+// A repo with one committed file, the state reinit accepts.
+const committedRepo = (): void => {
+  git("init", "-q", "-b", "canary")
+  git("config", "user.email", "t@t")
+  git("config", "user.name", "t")
+  writeFileSync(join(dir, "mine.txt"), "committed")
+  git("add", "-A")
+  git("commit", "-q", "-m", "init")
+}
+
+// Run the command and return what it printed (help goes through console.log, the flow through process.stdout.write) and what it threw.
+const run = async (argv: string[]): Promise<{ error: string; out: string }> => {
+  const chunks: string[] = []
+  const write = process.stdout.write
+  const log = console.log
+  process.stdout.write = ((chunk: string | Uint8Array) => {
+    chunks.push(String(chunk))
+    return true
+  }) as typeof process.stdout.write
+  console.log = (...parts: unknown[]) => {
+    chunks.push(`${parts.join(" ")}\n`)
+  }
+  let error = ""
+  try {
+    await reinit(argv)
+  } catch (err) {
+    error = err instanceof Error ? err.message : String(err)
+  } finally {
+    process.stdout.write = write
+    console.log = log
+  }
+  return { error, out: stripVTControlCharacters(chunks.join("")) }
+}
+
+const plain = (lines: string[]) => lines.map((line) => stripVTControlCharacters(line))
+
+beforeEach(() => {
+  dir = mkdtempSync(join(tmpdir(), "zs-reinit-"))
+})
+afterEach(() => {
+  rmSync(dir, { force: true, recursive: true })
+})
+
+describe("reinit refuses before it deletes anything", () => {
+  test("--help prints the usage and returns before the repo guard", async () => {
+    writeFileSync(join(dir, "mine.txt"), "not a repo")
+    const { error, out } = await run([dir, "--help"])
+    expect(error).toBe("")
+    expect(out).toContain("bunx zerostarter reinit [dir]")
+    expect(readFileSync(join(dir, "mine.txt"), "utf8")).toBe("not a repo")
+  })
+
+  test("a directory with no git repository is refused and points at init", async () => {
+    writeFileSync(join(dir, "mine.txt"), "not a repo")
+    const { error, out } = await run([dir, "--yes"])
+    expect(error).toContain("use init for a new project")
+    expect(out).toBe("")
+    expect(readFileSync(join(dir, "mine.txt"), "utf8")).toBe("not a repo")
+  })
+
+  test("an uncommitted edit is refused, since the wipe could not bring it back", async () => {
+    committedRepo()
+    writeFileSync(join(dir, "mine.txt"), "edited")
+    const { error, out } = await run([dir, "--yes"])
+    expect(error).toContain("uncommitted changes")
+    expect(out).toBe("")
+    expect(readFileSync(join(dir, "mine.txt"), "utf8")).toBe("edited")
+  })
+
+  test("an untracked file is refused too", async () => {
+    committedRepo()
+    writeFileSync(join(dir, "new.txt"), "untracked")
+    const { error, out } = await run([dir, "--yes"])
+    expect(error).toContain("uncommitted changes")
+    expect(out).toBe("")
+    expect(readFileSync(join(dir, "new.txt"), "utf8")).toBe("untracked")
+  })
+})
+
+describe("reinit next steps", () => {
+  test("ask for a database URL first while .env has none", () => {
+    expect(plain(nextSteps(false))).toEqual([
+      "set POSTGRES_URL in .env",
+      "bun run db:migrate",
+      "bun run dev",
+      "git push",
+    ])
+  })
+
+  test("skip the URL step once .env sets one", () => {
+    expect(plain(nextSteps(true))).toEqual(["bun run db:migrate", "bun run dev", "git push"])
+  })
+})