diff --git a/docs/configuration.md b/docs/configuration.md index cba3b13..7db005e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -297,8 +297,11 @@ process shutdown. ## Skills The required skill keys are `triage`, `planning`, and `implementation`. For new -repositories, `patchmill init` defaults them to local-only skill paths and adds -`.patchmill` plus `patchmill.config.json` to `.git/info/exclude`: +repositories, `patchmill init` defaults them to project-local skill paths. In an +interactive terminal, init asks whether to add generated config and skills to +git, add Patchmill files to `.gitignore`, or add Patchmill files to +`.git/info/exclude`. Non-interactive and `--yes` runs keep the files local by +adding `patchmill.config.json` and `.patchmill/` to `.git/info/exclude`: ```json { @@ -310,9 +313,11 @@ repositories, `patchmill init` defaults them to local-only skill paths and adds } ``` -Path-like skill references resolve relative to the config file directory. For -consistent Patchmill runs across local machines and CI, consider committing -`patchmill.config.json` and `.patchmill/skills/` explicitly. +Path-like skill references resolve relative to the config file directory. When +choosing **Add to git**, init stages `patchmill.config.json`, +`.patchmill/skills`, and `.gitignore`; `.gitignore` keeps `.patchmill/pi-agent`, +`.patchmill/runs`, and `.patchmill/triage-runs` local because they contain +machine-specific auth, session, and run output. ### Subagents diff --git a/docs/plans/2026-06-13-init-git-policy-prompt.md b/docs/plans/2026-06-13-init-git-policy-prompt.md new file mode 100644 index 0000000..e86b771 --- /dev/null +++ b/docs/plans/2026-06-13-init-git-policy-prompt.md @@ -0,0 +1,882 @@ +# Init Git Policy Prompt Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the `patchmill init` local-only warning with an interactive +git handling choice while keeping `.git/info/exclude` as the non-interactive +default. + +**Architecture:** Add a focused init git-policy helper that owns ignore/exclude +file edits, prompt normalization, and `git add` execution. Keep `main.ts` +responsible for orchestration: choose the policy, apply it, and print the +resulting message. + +**Tech Stack:** TypeScript, Node `node:test`, Node filesystem APIs, existing +`CommandRunner` abstraction. + +--- + +## File Structure + +- Create `src/cli/commands/init/git-policy.ts` + - Defines git policy choices and entries. + - Appends entries to `.gitignore` and `.git/info/exclude` without duplicates. + - Resolves `.git` directories and worktree gitdir files. + - Applies the chosen policy and returns a message for init output. +- Create `src/cli/commands/init/git-policy.test.ts` + - Unit tests for add-to-git, git-ignore, git-exclude, non-duplicate append + behavior, and missing git metadata warnings. +- Modify `src/cli/commands/init/main.ts` + - Replace direct `ensurePatchmillLocalExcludeEntries` use and broad + consistency warning with git policy selection and application. + - Add a small prompt path only for interactive runs without `--yes`. + - Inject an optional command runner for tests. +- Create `src/cli/commands/init/main-git-policy.test.ts` + - Integration tests for prompt choices and non-interactive defaults without + growing `main.test.ts` further. +- Modify `src/cli/commands/init/main.test.ts` + - Update existing assertions that expect the old warning or old exclude + message text. +- Modify `docs/configuration.md` + - Describe the interactive choice and the non-interactive `.git/info/exclude` + default. + +## Task 1: Add the git-policy helper with test-first coverage + +**Files:** + +- Create: `src/cli/commands/init/git-policy.test.ts` +- Create: `src/cli/commands/init/git-policy.ts` +- Read-only reference: `src/cli/commands/init/local-ignore.ts` +- Read-only reference: `src/cli/commands/triage/types.ts` + +- [ ] **Step 1: Write failing tests for helper behavior** + +Create `src/cli/commands/init/git-policy.test.ts` with this content: + +```ts +import assert from "node:assert/strict"; +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import type { CommandRunner } from "../triage/types.ts"; +import { applyInitGitPolicy, selectInitGitPolicy } from "./git-policy.ts"; + +async function tempRepo(options: { git?: boolean } = { git: true }) { + const repoRoot = await mkdtemp(join(tmpdir(), "patchmill-git-policy-")); + if (options.git !== false) { + await mkdir(join(repoRoot, ".git", "info"), { recursive: true }); + } + return repoRoot; +} + +function recordingRunner(calls: string[][] = []): CommandRunner { + return { + async run(command, args, options) { + calls.push([command, ...args, `cwd=${options?.cwd ?? ""}`]); + return { code: 0, stdout: "", stderr: "" }; + }, + }; +} + +test("selectInitGitPolicy defaults to git-exclude for non-interactive runs", async () => { + assert.equal( + await selectInitGitPolicy({ isInteractive: false, assumeYes: false }), + "exclude", + ); + assert.equal( + await selectInitGitPolicy({ isInteractive: true, assumeYes: true }), + "exclude", + ); +}); + +test("selectInitGitPolicy accepts add, ignore, and exclude prompt answers", async () => { + assert.equal( + await selectInitGitPolicy({ + isInteractive: true, + assumeYes: false, + prompt: async () => "1", + }), + "add", + ); + assert.equal( + await selectInitGitPolicy({ + isInteractive: true, + assumeYes: false, + prompt: async () => "ignore", + }), + "ignore", + ); + assert.equal( + await selectInitGitPolicy({ + isInteractive: true, + assumeYes: false, + prompt: async () => "", + }), + "exclude", + ); +}); + +test("applyInitGitPolicy add stages config, skills, and runtime ignore entries", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(calls), + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + ".patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + assert.deepEqual(calls, [ + [ + "git", + "add", + "patchmill.config.json", + ".patchmill/skills", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.match(result.message, /Added Patchmill config and skills to git/u); +}); + +test("applyInitGitPolicy ignore writes patchmill.config.json and .patchmill to .gitignore", async () => { + const repoRoot = await tempRepo(); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "ignore", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(result.message, /Added Patchmill files to .gitignore/u); +}); + +test("applyInitGitPolicy exclude writes patchmill.config.json and .patchmill to local exclude", async () => { + const repoRoot = await tempRepo(); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(result.message, /Added Patchmill files to .git\/info\/exclude/u); +}); + +test("applyInitGitPolicy does not duplicate existing ignore or exclude entries", async () => { + const repoRoot = await tempRepo(); + await writeFile( + join(repoRoot, ".gitignore"), + "node_modules\n.patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + await writeFile( + join(repoRoot, ".git", "info", "exclude"), + "node_modules\n.patchmill\npatchmill.config.json\n", + ); + + await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(), + }); + await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + "node_modules\n.patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "node_modules\n.patchmill\npatchmill.config.json\n", + ); +}); + +test("applyInitGitPolicy reports missing git metadata without failing init", async () => { + const repoRoot = await tempRepo({ git: false }); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.match( + result.message, + /Warning: Patchmill could not update .git\/info\/exclude/u, + ); + assert.match(result.message, /patchmill.config.json/u); + assert.match(result.message, /.patchmill\//u); +}); +``` + +- [ ] **Step 2: Run the helper tests to verify they fail because the module is + missing** + +Run: + +```bash +node --test src/cli/commands/init/git-policy.test.ts +``` + +Expected: FAIL with an import error for `./git-policy.ts`. + +- [ ] **Step 3: Implement the helper** + +Create `src/cli/commands/init/git-policy.ts` with this content: + +```ts +import { mkdir, readFile, stat, writeFile } from "node:fs/promises"; +import { isAbsolute, join, resolve } from "node:path"; +import type { CommandRunner } from "../triage/types.ts"; + +export type InitGitPolicy = "add" | "ignore" | "exclude"; + +export const PATCHMILL_GIT_IGNORE_ENTRIES = [ + "patchmill.config.json", + ".patchmill/", +] as const; + +export const PATCHMILL_ADD_TO_GIT_IGNORE_ENTRIES = [ + ".patchmill/pi-agent", + ".patchmill/runs", + ".patchmill/triage-runs", +] as const; + +export type InitGitPolicyResult = { + policy: InitGitPolicy; + message: string; +}; + +export type InitGitPolicyPrompt = (question: string) => Promise; + +function normalEntry(entry: string): string { + return entry.trim().replace(/\/+$/u, ""); +} + +function hasEntry(lines: string[], entry: string): boolean { + const wanted = normalEntry(entry); + return lines.some((line) => normalEntry(line) === wanted); +} + +async function appendEntries( + path: string, + entries: readonly string[], +): Promise { + let existing = ""; + try { + existing = await readFile(path, "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + + const lines = existing.split(/\r?\n/u); + const added = entries.filter((entry) => !hasEntry(lines, entry)); + if (added.length === 0) return []; + + const separator = + existing.length === 0 || existing.endsWith("\n") ? "" : "\n"; + await writeFile(path, `${existing}${separator}${added.join("\n")}\n`); + return added; +} + +async function resolveGitDir(repoRoot: string): Promise { + const dotGit = join(repoRoot, ".git"); + + try { + const dotGitStat = await stat(dotGit); + if (dotGitStat.isDirectory()) return dotGit; + if (!dotGitStat.isFile()) return undefined; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; + throw error; + } + + const dotGitContent = await readFile(dotGit, "utf8"); + const match = /^gitdir:\s*(.+?)\s*$/imu.exec(dotGitContent); + if (!match) return undefined; + + const gitDir = match[1] ?? ""; + return isAbsolute(gitDir) ? gitDir : resolve(repoRoot, gitDir); +} + +function formatEntries(entries: readonly string[]): string { + return entries.map((entry) => ` ${entry}`).join("\n"); +} + +export async function selectInitGitPolicy(options: { + isInteractive: boolean; + assumeYes: boolean; + prompt?: InitGitPolicyPrompt; +}): Promise { + if (!options.isInteractive || options.assumeYes || !options.prompt) { + return "exclude"; + } + + const answer = ( + await options.prompt( + [ + "How should Patchmill files be handled by git?", + " 1) Add config and skills to git", + " 2) Add Patchmill files to .gitignore", + " 3) Add Patchmill files to .git/info/exclude (local only)", + "Choose 1, 2, or 3 [3]: ", + ].join("\n"), + ) + ) + .trim() + .toLowerCase(); + + if (["1", "a", "add", "git", "add to git"].includes(answer)) return "add"; + if (["2", "i", "ignore", "gitignore", "git ignore"].includes(answer)) { + return "ignore"; + } + return "exclude"; +} + +export async function applyInitGitPolicy(options: { + repoRoot: string; + policy: InitGitPolicy; + runner: CommandRunner; +}): Promise { + if (options.policy === "add") { + const gitignorePath = join(options.repoRoot, ".gitignore"); + const added = await appendEntries( + gitignorePath, + PATCHMILL_ADD_TO_GIT_IGNORE_ENTRIES, + ); + const gitAdd = await options.runner.run( + "git", + ["add", "patchmill.config.json", ".patchmill/skills", ".gitignore"], + { cwd: options.repoRoot }, + ); + const gitAddMessage = + gitAdd.code === 0 + ? "Staged patchmill.config.json, .patchmill/skills, and .gitignore." + : `Warning: git add failed: ${gitAdd.stderr || gitAdd.stdout || "unknown error"}`; + const ignoreMessage = + added.length > 0 + ? `Added local Patchmill runtime directories to .gitignore:\n${formatEntries(added)}` + : "Local Patchmill runtime directories were already listed in .gitignore."; + return { + policy: options.policy, + message: [ + "Added Patchmill config and skills to git.", + gitAddMessage, + ignoreMessage, + ].join("\n"), + }; + } + + if (options.policy === "ignore") { + const added = await appendEntries( + join(options.repoRoot, ".gitignore"), + PATCHMILL_GIT_IGNORE_ENTRIES, + ); + return { + policy: options.policy, + message: + added.length > 0 + ? `Added Patchmill files to .gitignore:\n${formatEntries(added)}` + : "Patchmill files were already listed in .gitignore.", + }; + } + + const gitDir = await resolveGitDir(options.repoRoot); + const excludePath = gitDir + ? join(gitDir, "info", "exclude") + : join(options.repoRoot, ".git", "info", "exclude"); + if (!gitDir) { + return { + policy: options.policy, + message: [ + "Warning: Patchmill could not update .git/info/exclude because this directory is not inside a git repository.", + "Add these entries manually to keep Patchmill files local:", + formatEntries(PATCHMILL_GIT_IGNORE_ENTRIES), + ].join("\n"), + }; + } + + await mkdir(join(gitDir, "info"), { recursive: true }); + const added = await appendEntries(excludePath, PATCHMILL_GIT_IGNORE_ENTRIES); + return { + policy: options.policy, + message: + added.length > 0 + ? `Added Patchmill files to .git/info/exclude:\n${formatEntries(added)}` + : "Patchmill files were already listed in .git/info/exclude.", + }; +} +``` + +- [ ] **Step 4: Run helper tests to verify green** + +Run: + +```bash +node --test src/cli/commands/init/git-policy.test.ts +``` + +Expected: PASS. + +- [ ] **Step 5: Commit helper** + +Run: + +```bash +git add src/cli/commands/init/git-policy.ts src/cli/commands/init/git-policy.test.ts +git commit -m "feat(init): add git policy helper" +``` + +## Task 2: Wire the prompt into `patchmill init` + +**Files:** + +- Modify: `src/cli/commands/init/main.ts` +- Create: `src/cli/commands/init/main-git-policy.test.ts` +- Modify: `src/cli/commands/init/main.test.ts` + +- [ ] **Step 1: Write failing init integration tests** + +Create `src/cli/commands/init/main-git-policy.test.ts` with this content: + +```ts +import assert from "node:assert/strict"; +import { mkdir, mkdtemp, readFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import type { CommandRunner } from "../triage/types.ts"; +import { runInit } from "./main.ts"; + +async function tempRepo(): Promise { + const repoRoot = await mkdtemp(join(tmpdir(), "patchmill-init-git-policy-")); + await mkdir(join(repoRoot, ".git", "info"), { recursive: true }); + return repoRoot; +} + +function missingPiReadiness() { + return { + status: "missing" as const, + message: "Pi did not report any provider/model with configured auth.", + models: [], + }; +} + +async function failingPiSmokeTest() { + return { + status: "fail" as const, + message: "Pi could not complete the provider smoke test.", + command: + "pi --no-session --no-context-files --no-prompt-templates -p Reply with PATCHMILL_PI_OK and nothing else.", + details: "missing key", + }; +} + +function runner(calls: string[][]): CommandRunner { + return { + async run(command, args, options) { + calls.push([command, ...args, `cwd=${options?.cwd ?? ""}`]); + return { code: 0, stdout: "", stderr: "" }; + }, + }; +} + +async function runInitForGitPolicy( + repoRoot: string, + options: { + args?: string[]; + isInteractive: boolean; + promptAnswer?: string; + calls?: string[][]; + }, +) { + const stdout: string[] = []; + await runInit( + options.args ?? [], + repoRoot, + { + stdout: (line) => stdout.push(line), + stderr: () => undefined, + }, + { + detectPiReadiness: missingPiReadiness, + runPiSmokeTest: failingPiSmokeTest, + isInteractive: options.isInteractive, + prompt: async () => options.promptAnswer ?? "", + commandRunner: runner(options.calls ?? []), + setupLabels: async () => ({ + status: "skipped", + message: "Label setup skipped.", + }), + }, + ); + return stdout.join("\n"); +} + +test("interactive init add-to-git stages config, skills, and gitignore", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + + const output = await runInitForGitPolicy(repoRoot, { + isInteractive: true, + promptAnswer: "1", + calls, + }); + + assert.deepEqual(calls, [ + [ + "git", + "add", + "patchmill.config.json", + ".patchmill/skills", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + ".patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + assert.match(output, /Added Patchmill config and skills to git/u); + assert.doesNotMatch(output, /local-only by default/u); +}); + +test("interactive init git-ignore writes config and .patchmill to .gitignore", async () => { + const repoRoot = await tempRepo(); + + const output = await runInitForGitPolicy(repoRoot, { + isInteractive: true, + promptAnswer: "2", + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(output, /Added Patchmill files to .gitignore/u); + assert.doesNotMatch(output, /local-only by default/u); +}); + +test("interactive init git-exclude writes config and .patchmill to local exclude", async () => { + const repoRoot = await tempRepo(); + + const output = await runInitForGitPolicy(repoRoot, { + isInteractive: true, + promptAnswer: "3", + }); + + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(output, /Added Patchmill files to .git\/info\/exclude/u); +}); + +test("non-interactive and --yes init choose git-exclude without prompting", async () => { + const nonInteractiveRoot = await tempRepo(); + const yesRoot = await tempRepo(); + let prompted = false; + + await runInitForGitPolicy(nonInteractiveRoot, { + isInteractive: false, + promptAnswer: "1", + }); + await runInit( + ["--yes"], + yesRoot, + { + stdout: () => undefined, + stderr: () => undefined, + }, + { + detectPiReadiness: missingPiReadiness, + runPiSmokeTest: failingPiSmokeTest, + isInteractive: true, + prompt: async () => { + prompted = true; + return "1"; + }, + commandRunner: runner([]), + setupLabels: async () => ({ + status: "skipped", + message: "Label setup skipped.", + }), + }, + ); + + assert.equal(prompted, false); + assert.equal( + await readFile(join(nonInteractiveRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.equal( + await readFile(join(yesRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); +}); +``` + +- [ ] **Step 2: Run the new integration tests to verify they fail on missing + wiring** + +Run: + +```bash +node --test src/cli/commands/init/main-git-policy.test.ts +``` + +Expected: FAIL because `runInit` does not accept `commandRunner`, does not +prompt for git policy, and still writes old exclude entries. + +- [ ] **Step 3: Modify `main.ts` imports and options** + +In `src/cli/commands/init/main.ts`, replace: + +```ts +import { ensurePatchmillLocalExcludeEntries } from "./local-ignore.ts"; +``` + +with: + +```ts +import { applyInitGitPolicy, selectInitGitPolicy } from "./git-policy.ts"; +import type { CommandRunner } from "../triage/types.ts"; +``` + +Then add this option to the `runInit` options type: + +```ts + commandRunner?: CommandRunner; +``` + +- [ ] **Step 4: Replace local exclude and consistency warning orchestration** + +In `src/cli/commands/init/main.ts`, replace this block: + +```ts +const localExclude = await ensurePatchmillLocalExcludeEntries(config.repoRoot); +const localExcludeMessage = localExclude.skipped + ? `Warning: Patchmill could not update .git/info/exclude (${localExclude.skipped}).\nAdd .patchmill and patchmill.config.json to your local git excludes to keep the worktree clean.` + : localExclude.added.length > 0 + ? `Added Patchmill local files to .git/info/exclude:\n ${localExclude.added.join("\n ")}` + : "Patchmill local files were already ignored by .git/info/exclude."; +const consistencyWarning = + "Warning: Patchmill config and skills are local-only by default. For consistent Patchmill runs across local machines and CI, consider committing patchmill.config.json and .patchmill/skills/ explicitly."; +const isInteractive = options.isInteractive ?? defaultStdin.isTTY; +``` + +with: + +```ts +const isInteractive = options.isInteractive ?? defaultStdin.isTTY; +const gitPolicy = await selectInitGitPolicy({ + isInteractive, + assumeYes: config.yes, + prompt: options.prompt ?? defaultPrompt, +}); +const gitPolicyResult = await applyInitGitPolicy({ + repoRoot: config.repoRoot, + policy: gitPolicy, + runner: options.commandRunner ?? createCommandRunner(), +}); +``` + +Then replace the output segment: + +```ts +\n\n${localExcludeMessage}\n\n${consistencyWarning}\n\n${skillsMessage} +``` + +with: + +```ts +\n\n${gitPolicyResult.message}\n\n${skillsMessage} +``` + +- [ ] **Step 5: Run the new integration tests to verify green** + +Run: + +```bash +node --test src/cli/commands/init/main-git-policy.test.ts +``` + +Expected: PASS. + +- [ ] **Step 6: Update existing main test assertions** + +In `src/cli/commands/init/main.test.ts`, update expectations in +`runInit installs project-local skills by default`: + +```ts +assert.match( + stdout.join("\n"), + /Added Patchmill files to \.git\/info\/exclude/, +); +assert.match( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + /patchmill\.config\.json\n\.patchmill\/\n/u, +); +assert.doesNotMatch(stdout.join("\n"), /local-only by default/u); +``` + +Remove the old assertion that matches +`Warning: Patchmill config and skills are local-only by default`. + +In +`runInit preserves existing local exclude entries and does not touch gitignore`, +update the expected exclude content to: + +```ts +assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "node_modules\n.patchmill\npatchmill.config.json\n", +); +``` + +- [ ] **Step 7: Run init tests** + +Run: + +```bash +node --test src/cli/commands/init/git-policy.test.ts src/cli/commands/init/main-git-policy.test.ts src/cli/commands/init/main.test.ts +``` + +Expected: PASS. + +- [ ] **Step 8: Commit init wiring** + +Run: + +```bash +git add src/cli/commands/init/main.ts src/cli/commands/init/main.test.ts src/cli/commands/init/main-git-policy.test.ts +git commit -m "feat(init): prompt for git policy" +``` + +## Task 3: Update documentation and run final verification + +**Files:** + +- Modify: `docs/configuration.md` + +- [ ] **Step 1: Update docs text for init git handling** + +In `docs/configuration.md`, replace this paragraph under `## Skills`: + +```md +The required skill keys are `triage`, `planning`, and `implementation`. For new +repositories, `patchmill init` defaults them to local-only skill paths and adds +`.patchmill` plus `patchmill.config.json` to `.git/info/exclude`: +``` + +with: + +```md +The required skill keys are `triage`, `planning`, and `implementation`. For new +repositories, `patchmill init` defaults them to project-local skill paths. In an +interactive terminal, init asks whether to add generated config and skills to +git, add Patchmill files to `.gitignore`, or add Patchmill files to +`.git/info/exclude`. Non-interactive and `--yes` runs keep the files local by +adding `patchmill.config.json` and `.patchmill/` to `.git/info/exclude`: +``` + +Then replace this paragraph: + +```md +Path-like skill references resolve relative to the config file directory. For +consistent Patchmill runs across local machines and CI, consider committing +`patchmill.config.json` and `.patchmill/skills/` explicitly. +``` + +with: + +```md +Path-like skill references resolve relative to the config file directory. When +choosing **Add to git**, init stages `patchmill.config.json`, +`.patchmill/skills`, and `.gitignore`; `.gitignore` keeps `.patchmill/pi-agent`, +`.patchmill/runs`, and `.patchmill/triage-runs` local because they contain +machine-specific auth, session, and run output. +``` + +- [ ] **Step 2: Run docs verification** + +Run: + +```bash +npx markdownlint-cli2 docs/configuration.md docs/specs/2026-06-13-init-git-policy-prompt-design.md docs/plans/2026-06-13-init-git-policy-prompt.md +``` + +Expected: PASS. + +- [ ] **Step 3: Run targeted tests** + +Run: + +```bash +node --test src/cli/commands/init/git-policy.test.ts src/cli/commands/init/main-git-policy.test.ts src/cli/commands/init/main.test.ts +``` + +Expected: PASS. + +- [ ] **Step 4: Run the full test suite** + +Run: + +```bash +npm test +``` + +Expected: PASS. + +- [ ] **Step 5: Run TypeScript and formatting checks** + +Run: + +```bash +npm run lint:ts +npm run format:check +``` + +Expected: both commands PASS. + +- [ ] **Step 6: Commit docs and verification follow-up** + +Run: + +```bash +git add docs/configuration.md docs/plans/2026-06-13-init-git-policy-prompt.md +git commit -m "docs(init): document git policy prompt" +``` + +## Self-Review + +- Spec coverage: Task 1 covers the three policy effects, duplicate handling, and + missing git metadata warnings. Task 2 covers interactive prompt wiring plus + non-interactive and `--yes` defaults. Task 3 covers documentation and final + verification. +- Placeholder scan: The plan contains no placeholder markers or deferred + behavior. +- Type consistency: The plan consistently uses `InitGitPolicy`, + `InitGitPolicyResult`, `selectInitGitPolicy`, `applyInitGitPolicy`, and + `CommandRunner`. diff --git a/docs/specs/2026-06-13-init-git-policy-prompt-design.md b/docs/specs/2026-06-13-init-git-policy-prompt-design.md new file mode 100644 index 0000000..4eae714 --- /dev/null +++ b/docs/specs/2026-06-13-init-git-policy-prompt-design.md @@ -0,0 +1,76 @@ +# `patchmill init` Git Policy Prompt Design + +## Context + +`patchmill init` currently installs project-local skills by default, writes +`patchmill.config.json`, automatically adds Patchmill local paths to +`.git/info/exclude`, and prints a broad warning that config and skills are +local-only by default. + +That warning leaves the user to decide what to do next. Instead, interactive +initialization should ask how the generated Patchmill files should be handled by +git. + +## Goals + +- Replace the local-only warning with an interactive git-policy choice. +- Preserve the current safe default for non-interactive and `--yes` runs. +- Let users commit reusable Patchmill configuration and skills when they choose + to do so. +- Always protect local runtime/auth/session directories when the user chooses to + add config and skills to git. + +## Behavior + +After `patchmill init` creates `patchmill.config.json` and installs or validates +skills, interactive runs prompt for one of three choices: + +1. **Add to git** + - Append these entries to `.gitignore` if missing: + - `.patchmill/pi-agent` + - `.patchmill/runs` + - `.patchmill/triage-runs` + - Run `git add patchmill.config.json .patchmill/skills .gitignore`. + - Report the staged files and ignored local runtime directories. +2. **Git ignore** + - Append these entries to `.gitignore` if missing: + - `patchmill.config.json` + - `.patchmill/` + - Report the `.gitignore` update. +3. **Git exclude** + - Append these entries to `.git/info/exclude` if missing: + - `patchmill.config.json` + - `.patchmill/` + - Report the local exclude update. + +Non-interactive runs and `--yes` keep the current safe default by choosing **Git +exclude** automatically. + +If the repository has no writable git metadata, Patchmill should still finish +initialization and print a clear warning explaining which entries the user +should add manually. + +## Implementation Notes + +- Move git-policy file updates into a focused helper module so `main.ts` stays + responsible for CLI orchestration rather than low-level file edits. +- Reuse the existing prompt dependency used by init label setup for simple + terminal prompts. +- Use deterministic, line-oriented append logic that avoids duplicate entries. +- Run `git add` through the existing command-runner path so tests can inject a + fake runner or command behavior. +- Keep existing skill installation modes unchanged. + +## Testing + +Automated tests should cover: + +- Interactive **Add to git** updates `.gitignore` with runtime/auth/session + entries and runs `git add patchmill.config.json .patchmill/skills .gitignore`. +- Interactive **Git ignore** appends `patchmill.config.json` and `.patchmill/` + to `.gitignore`. +- Interactive **Git exclude** appends `patchmill.config.json` and `.patchmill/` + to `.git/info/exclude`. +- Non-interactive and `--yes` runs choose **Git exclude** without prompting. +- Existing ignore/exclude entries are not duplicated. +- Missing git metadata produces an actionable warning instead of failing init. diff --git a/src/cli/commands/init/git-policy.test.ts b/src/cli/commands/init/git-policy.test.ts new file mode 100644 index 0000000..3935d6a --- /dev/null +++ b/src/cli/commands/init/git-policy.test.ts @@ -0,0 +1,286 @@ +import assert from "node:assert/strict"; +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import type { CommandRunner } from "../triage/types.ts"; +import { applyInitGitPolicy, selectInitGitPolicy } from "./git-policy.ts"; + +async function tempRepo(options: { git?: boolean } = { git: true }) { + const repoRoot = await mkdtemp(join(tmpdir(), "patchmill-git-policy-")); + if (options.git !== false) { + await mkdir(join(repoRoot, ".git", "info"), { recursive: true }); + } + return repoRoot; +} + +function recordingRunner(calls: string[][] = []): CommandRunner { + return { + async run(command, args, options) { + calls.push([command, ...args, `cwd=${options?.cwd ?? ""}`]); + return { code: 0, stdout: "", stderr: "" }; + }, + }; +} + +test("selectInitGitPolicy defaults to git-exclude for non-interactive runs", async () => { + assert.equal( + await selectInitGitPolicy({ isInteractive: false, assumeYes: false }), + "exclude", + ); + assert.equal( + await selectInitGitPolicy({ isInteractive: true, assumeYes: true }), + "exclude", + ); +}); + +test("selectInitGitPolicy accepts add, ignore, and exclude prompt answers", async () => { + assert.equal( + await selectInitGitPolicy({ + isInteractive: true, + assumeYes: false, + prompt: async () => "1", + }), + "add", + ); + assert.equal( + await selectInitGitPolicy({ + isInteractive: true, + assumeYes: false, + prompt: async () => "ignore", + }), + "ignore", + ); + assert.equal( + await selectInitGitPolicy({ + isInteractive: true, + assumeYes: false, + prompt: async () => "", + }), + "exclude", + ); +}); + +test("applyInitGitPolicy add stages config, skills, and runtime ignore entries", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + await writeFile(join(repoRoot, "patchmill.config.json"), "{}\n"); + await mkdir(join(repoRoot, ".patchmill", "skills"), { recursive: true }); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(calls), + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + ".patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + assert.deepEqual(calls, [ + [ + "git", + "add", + "-f", + "patchmill.config.json", + ".patchmill/skills", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.match(result.message, /Added Patchmill config and skills to git/u); +}); + +test("applyInitGitPolicy add omits missing skills directory", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + await writeFile(join(repoRoot, "patchmill.config.json"), "{}\n"); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(calls), + }); + + assert.deepEqual(calls, [ + [ + "git", + "add", + "-f", + "patchmill.config.json", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.match(result.message, /Staged patchmill.config.json and .gitignore/u); + assert.doesNotMatch(result.message, /.patchmill\/skills/u); +}); + +test("applyInitGitPolicy add stages provided repo-local skill roots", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + await writeFile(join(repoRoot, "patchmill.config.json"), "{}\n"); + await mkdir(join(repoRoot, "custom-skills"), { recursive: true }); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(calls), + skillRoots: ["custom-skills"], + }); + + assert.deepEqual(calls, [ + [ + "git", + "add", + "-f", + "patchmill.config.json", + "custom-skills", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.match(result.message, /Added Patchmill config and skills to git/u); +}); + +test("applyInitGitPolicy add force-stages skills when .patchmill is ignored", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + await writeFile(join(repoRoot, "patchmill.config.json"), "{}\n"); + await mkdir(join(repoRoot, ".patchmill", "skills"), { recursive: true }); + await writeFile(join(repoRoot, ".gitignore"), ".patchmill/\n"); + + await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(calls), + }); + + assert.deepEqual(calls[0]?.slice(0, 4), [ + "git", + "add", + "-f", + "patchmill.config.json", + ]); + assert.ok(calls[0]?.includes(".patchmill/skills")); +}); + +test("applyInitGitPolicy ignore writes patchmill.config.json and .patchmill to .gitignore", async () => { + const repoRoot = await tempRepo(); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "ignore", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(result.message, /Added Patchmill files to .gitignore/u); +}); + +test("applyInitGitPolicy exclude writes patchmill.config.json and .patchmill to local exclude", async () => { + const repoRoot = await tempRepo(); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(result.message, /Added Patchmill files to .git\/info\/exclude/u); +}); + +test("applyInitGitPolicy does not duplicate existing ignore or exclude entries", async () => { + const repoRoot = await tempRepo(); + await writeFile( + join(repoRoot, ".gitignore"), + "node_modules\n.patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + await writeFile( + join(repoRoot, ".git", "info", "exclude"), + "node_modules\n.patchmill\npatchmill.config.json\n", + ); + + await applyInitGitPolicy({ + repoRoot, + policy: "add", + runner: recordingRunner(), + }); + await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + "node_modules\n.patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "node_modules\n.patchmill\npatchmill.config.json\n", + ); +}); + +test("applyInitGitPolicy treats root-anchored exclude entries as duplicates", async () => { + const repoRoot = await tempRepo(); + await writeFile( + join(repoRoot, ".git", "info", "exclude"), + "node_modules\n/.patchmill/\n/patchmill.config.json\n", + ); + + await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "node_modules\n/.patchmill/\n/patchmill.config.json\n", + ); +}); + +test("applyInitGitPolicy reports missing git metadata without failing init", async () => { + const repoRoot = await tempRepo({ git: false }); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.match( + result.message, + /Warning: Patchmill could not update .git\/info\/exclude/u, + ); + assert.match(result.message, /patchmill.config.json/u); + assert.match(result.message, /.patchmill\//u); +}); + +test("applyInitGitPolicy reports local exclude write failures without failing init", async () => { + const repoRoot = await mkdtemp(join(tmpdir(), "patchmill-git-policy-")); + await mkdir(join(repoRoot, ".git"), { recursive: true }); + await writeFile(join(repoRoot, ".git", "info"), "not a directory\n"); + + const result = await applyInitGitPolicy({ + repoRoot, + policy: "exclude", + runner: recordingRunner(), + }); + + assert.match( + result.message, + /Warning: Patchmill could not update .git\/info\/exclude/u, + ); + assert.match(result.message, /not a directory|EEXIST/u); + assert.match(result.message, /patchmill.config.json/u); + assert.match(result.message, /.patchmill\//u); +}); diff --git a/src/cli/commands/init/git-policy.ts b/src/cli/commands/init/git-policy.ts new file mode 100644 index 0000000..b9f9eee --- /dev/null +++ b/src/cli/commands/init/git-policy.ts @@ -0,0 +1,251 @@ +import { mkdir, readFile, stat, writeFile } from "node:fs/promises"; +import { isAbsolute, join, resolve } from "node:path"; +import type { CommandRunner } from "../triage/types.ts"; + +export type InitGitPolicy = "add" | "ignore" | "exclude"; + +export const PATCHMILL_GIT_IGNORE_ENTRIES = [ + "patchmill.config.json", + ".patchmill/", +] as const; + +export const PATCHMILL_ADD_TO_GIT_IGNORE_ENTRIES = [ + ".patchmill/pi-agent", + ".patchmill/runs", + ".patchmill/triage-runs", +] as const; + +export type InitGitPolicyResult = { + policy: InitGitPolicy; + message: string; +}; + +export type InitGitPolicyPrompt = (question: string) => Promise; + +function normalEntry(entry: string): string { + return entry.trim().replace(/^\//u, "").replace(/\/+$/u, ""); +} + +function hasEntry(lines: string[], entry: string): boolean { + const wanted = normalEntry(entry); + return lines.some((line) => normalEntry(line) === wanted); +} + +async function appendEntries( + path: string, + entries: readonly string[], +): Promise { + let existing = ""; + try { + existing = await readFile(path, "utf8"); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + + const lines = existing.split(/\r?\n/u); + const added = entries.filter((entry) => !hasEntry(lines, entry)); + if (added.length === 0) return []; + + const separator = + existing.length === 0 || existing.endsWith("\n") ? "" : "\n"; + await writeFile(path, `${existing}${separator}${added.join("\n")}\n`); + return added; +} + +async function resolveGitDir(repoRoot: string): Promise { + const dotGit = join(repoRoot, ".git"); + + try { + const dotGitStat = await stat(dotGit); + if (dotGitStat.isDirectory()) return dotGit; + if (!dotGitStat.isFile()) return undefined; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; + throw error; + } + + const dotGitContent = await readFile(dotGit, "utf8"); + const match = /^gitdir:\s*(.+?)\s*$/imu.exec(dotGitContent); + if (!match) return undefined; + + const gitDir = match[1] ?? ""; + return isAbsolute(gitDir) ? gitDir : resolve(repoRoot, gitDir); +} + +function formatEntries(entries: readonly string[]): string { + return entries.map((entry) => ` ${entry}`).join("\n"); +} + +function formatPathList(paths: readonly string[]): string { + if (paths.length <= 1) return paths[0] ?? "nothing"; + if (paths.length === 2) return `${paths[0]} and ${paths[1]}`; + return `${paths.slice(0, -1).join(", ")}, and ${paths.at(-1)}`; +} + +function safeRelativePath(path: string): boolean { + return ( + path !== "." && + path !== ".." && + !isAbsolute(path) && + !path.startsWith("../") + ); +} + +async function existingPaths( + repoRoot: string, + paths: readonly string[], +): Promise { + const uniquePaths = [...new Set(paths)].filter(safeRelativePath); + const existing: string[] = []; + for (const path of uniquePaths) { + try { + await stat(join(repoRoot, path)); + existing.push(path); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error; + } + } + return existing; +} + +function manualExcludeWarning(reason: string): string { + return [ + `Warning: Patchmill could not update .git/info/exclude (${reason}).`, + "Add these entries manually to keep Patchmill files local:", + formatEntries(PATCHMILL_GIT_IGNORE_ENTRIES), + ].join("\n"); +} + +export async function selectInitGitPolicy(options: { + isInteractive: boolean; + assumeYes: boolean; + prompt?: InitGitPolicyPrompt; +}): Promise { + if (!options.isInteractive || options.assumeYes || !options.prompt) { + return "exclude"; + } + + const answer = ( + await options.prompt( + [ + "How should Patchmill files be handled by git?", + " 1) Add config and skills to git", + " 2) Add Patchmill files to .gitignore", + " 3) Add Patchmill files to .git/info/exclude (local only)", + "Choose 1, 2, or 3 [3]: ", + ].join("\n"), + ) + ) + .trim() + .toLowerCase(); + + if (["1", "a", "add", "git", "add to git"].includes(answer)) return "add"; + if (["2", "i", "ignore", "gitignore", "git ignore"].includes(answer)) { + return "ignore"; + } + return "exclude"; +} + +export async function applyInitGitPolicy(options: { + repoRoot: string; + policy: InitGitPolicy; + runner: CommandRunner; + skillRoots?: readonly string[]; +}): Promise { + if (options.policy === "add") { + const gitignorePath = join(options.repoRoot, ".gitignore"); + const added = await appendEntries( + gitignorePath, + PATCHMILL_ADD_TO_GIT_IGNORE_ENTRIES, + ); + const skillRoots = options.skillRoots ?? [".patchmill/skills"]; + const pathsToStage = await existingPaths(options.repoRoot, [ + "patchmill.config.json", + ...skillRoots, + ".gitignore", + ]); + const gitAdd = + pathsToStage.length > 0 + ? await options.runner.run("git", ["add", "-f", ...pathsToStage], { + cwd: options.repoRoot, + }) + : undefined; + const gitAddMessage = !gitAdd + ? "No Patchmill files were available to stage." + : gitAdd.code === 0 + ? `Staged ${formatPathList(pathsToStage)}.` + : `Warning: git add failed: ${gitAdd.stderr || gitAdd.stdout || "unknown error"}`; + const ignoreMessage = + added.length > 0 + ? `Added local Patchmill runtime directories to .gitignore:\n${formatEntries(added)}` + : "Local Patchmill runtime directories were already listed in .gitignore."; + const stagedSkills = pathsToStage.some((path) => skillRoots.includes(path)); + const addSummary = + gitAdd?.code === 0 + ? stagedSkills + ? "Added Patchmill config and skills to git." + : "Added Patchmill config to git." + : "Warning: Patchmill could not add files to git."; + return { + policy: options.policy, + message: [addSummary, gitAddMessage, ignoreMessage].join("\n"), + }; + } + + if (options.policy === "ignore") { + const added = await appendEntries( + join(options.repoRoot, ".gitignore"), + PATCHMILL_GIT_IGNORE_ENTRIES, + ); + return { + policy: options.policy, + message: + added.length > 0 + ? `Added Patchmill files to .gitignore:\n${formatEntries(added)}` + : "Patchmill files were already listed in .gitignore.", + }; + } + + let gitDir: string | undefined; + try { + gitDir = await resolveGitDir(options.repoRoot); + } catch (error) { + return { + policy: options.policy, + message: manualExcludeWarning( + error instanceof Error ? error.message : String(error), + ), + }; + } + if (!gitDir) { + return { + policy: options.policy, + message: manualExcludeWarning( + "not inside a git repository with a local exclude file", + ), + }; + } + + let added: string[]; + try { + await mkdir(join(gitDir, "info"), { recursive: true }); + added = await appendEntries( + join(gitDir, "info", "exclude"), + PATCHMILL_GIT_IGNORE_ENTRIES, + ); + } catch (error) { + return { + policy: options.policy, + message: manualExcludeWarning( + error instanceof Error ? error.message : String(error), + ), + }; + } + return { + policy: options.policy, + message: + added.length > 0 + ? `Added Patchmill files to .git/info/exclude:\n${formatEntries(added)}` + : "Patchmill files were already listed in .git/info/exclude.", + }; +} diff --git a/src/cli/commands/init/main-git-policy.test.ts b/src/cli/commands/init/main-git-policy.test.ts new file mode 100644 index 0000000..88d3351 --- /dev/null +++ b/src/cli/commands/init/main-git-policy.test.ts @@ -0,0 +1,253 @@ +import assert from "node:assert/strict"; +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import type { CommandRunner } from "../triage/types.ts"; +import { runInit } from "./main.ts"; + +async function tempRepo(): Promise { + const repoRoot = await mkdtemp(join(tmpdir(), "patchmill-init-git-policy-")); + await mkdir(join(repoRoot, ".git", "info"), { recursive: true }); + return repoRoot; +} + +async function writeSkill(repoRoot: string, skillRoot: string, name: string) { + const dir = join(repoRoot, skillRoot, name); + await mkdir(dir, { recursive: true }); + await writeFile( + join(dir, "SKILL.md"), + `---\nname: ${name}\ndescription: Test skill\n---\n`, + ); +} + +function missingPiReadiness() { + return { + status: "missing" as const, + message: "Pi did not report any provider/model with configured auth.", + models: [], + }; +} + +async function failingPiSmokeTest() { + return { + status: "fail" as const, + message: "Pi could not complete the provider smoke test.", + command: + "pi --no-session --no-context-files --no-prompt-templates -p Reply with PATCHMILL_PI_OK and nothing else.", + details: "missing key", + }; +} + +async function incompletePiSetup() { + return { + status: "incomplete" as const, + readiness: missingPiReadiness(), + selection: { + status: "unavailable" as const, + reason: "not-ready" as const, + message: "Pi provider/model setup is incomplete.", + }, + smoke: await failingPiSmokeTest(), + }; +} + +function runner(calls: string[][]): CommandRunner { + return { + async run(command, args, options) { + calls.push([command, ...args, `cwd=${options?.cwd ?? ""}`]); + return { code: 0, stdout: "", stderr: "" }; + }, + }; +} + +async function runInitForGitPolicy( + repoRoot: string, + options: { + args?: string[]; + isInteractive: boolean; + promptAnswer?: string; + calls?: string[][]; + }, +) { + const stdout: string[] = []; + await runInit( + options.args ?? [], + repoRoot, + { + stdout: (line) => stdout.push(line), + stderr: () => undefined, + }, + { + detectPiReadiness: missingPiReadiness, + runPiSmokeTest: failingPiSmokeTest, + resolvePiInitSetup: incompletePiSetup, + isInteractive: options.isInteractive, + prompt: async () => options.promptAnswer ?? "", + commandRunner: runner(options.calls ?? []), + setupLabels: async () => ({ + status: "skipped", + message: "Label setup skipped.", + }), + }, + ); + return stdout.join("\n"); +} + +test("interactive init add-to-git stages config, skills, and gitignore", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + + const output = await runInitForGitPolicy(repoRoot, { + isInteractive: true, + promptAnswer: "1", + calls, + }); + + assert.deepEqual(calls, [ + [ + "git", + "add", + "-f", + "patchmill.config.json", + ".patchmill/skills", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + ".patchmill/pi-agent\n.patchmill/runs\n.patchmill/triage-runs\n", + ); + assert.match(output, /Added Patchmill config and skills to git/u); + assert.doesNotMatch(output, /local-only by default/u); +}); + +test("interactive init add-to-git with no skills stages config and gitignore only", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + + const output = await runInitForGitPolicy(repoRoot, { + args: ["--skills", "none"], + isInteractive: true, + promptAnswer: "1", + calls, + }); + + assert.deepEqual(calls, [ + [ + "git", + "add", + "-f", + "patchmill.config.json", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.match(output, /Added Patchmill config to git/u); + assert.doesNotMatch(output, /.patchmill\/skills/u); +}); + +test("interactive init add-to-git with path skills stages the provided skill root", async () => { + const repoRoot = await tempRepo(); + const calls: string[][] = []; + await writeSkill(repoRoot, "custom-skills", "patchmill-issue-triage"); + await writeSkill(repoRoot, "custom-skills", "writing-plans"); + await writeSkill(repoRoot, "custom-skills", "subagent-driven-development"); + + const output = await runInitForGitPolicy(repoRoot, { + args: ["--skills", "path:custom-skills"], + isInteractive: true, + promptAnswer: "1", + calls, + }); + + assert.deepEqual(calls, [ + [ + "git", + "add", + "-f", + "patchmill.config.json", + "custom-skills", + ".gitignore", + `cwd=${repoRoot}`, + ], + ]); + assert.match(output, /Added Patchmill config and skills to git/u); + assert.doesNotMatch(output, /.patchmill\/skills/u); +}); + +test("interactive init git-ignore writes config and .patchmill to .gitignore", async () => { + const repoRoot = await tempRepo(); + + const output = await runInitForGitPolicy(repoRoot, { + isInteractive: true, + promptAnswer: "2", + }); + + assert.equal( + await readFile(join(repoRoot, ".gitignore"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(output, /Added Patchmill files to .gitignore/u); + assert.doesNotMatch(output, /local-only by default/u); +}); + +test("interactive init git-exclude writes config and .patchmill to local exclude", async () => { + const repoRoot = await tempRepo(); + + const output = await runInitForGitPolicy(repoRoot, { + isInteractive: true, + promptAnswer: "3", + }); + + assert.equal( + await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.match(output, /Added Patchmill files to .git\/info\/exclude/u); +}); + +test("non-interactive and --yes init choose git-exclude without prompting", async () => { + const nonInteractiveRoot = await tempRepo(); + const yesRoot = await tempRepo(); + let prompted = false; + + await runInitForGitPolicy(nonInteractiveRoot, { + isInteractive: false, + promptAnswer: "1", + }); + await runInit( + ["--yes"], + yesRoot, + { + stdout: () => undefined, + stderr: () => undefined, + }, + { + detectPiReadiness: missingPiReadiness, + runPiSmokeTest: failingPiSmokeTest, + resolvePiInitSetup: incompletePiSetup, + isInteractive: true, + prompt: async () => { + prompted = true; + return "1"; + }, + commandRunner: runner([]), + setupLabels: async () => ({ + status: "skipped", + message: "Label setup skipped.", + }), + }, + ); + + assert.equal(prompted, false); + assert.equal( + await readFile(join(nonInteractiveRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); + assert.equal( + await readFile(join(yesRoot, ".git", "info", "exclude"), "utf8"), + "patchmill.config.json\n.patchmill/\n", + ); +}); diff --git a/src/cli/commands/init/main.test.ts b/src/cli/commands/init/main.test.ts index f4d6436..029d257 100644 --- a/src/cli/commands/init/main.test.ts +++ b/src/cli/commands/init/main.test.ts @@ -137,16 +137,13 @@ test("runInit installs project-local skills by default", async () => { assert.match(stdout.join("\n"), /Installed project-local skills/); assert.match( stdout.join("\n"), - /Added Patchmill local files to \.git\/info\/exclude/, + /Added Patchmill files to \.git\/info\/exclude/, ); assert.match( await readFile(join(repoRoot, ".git", "info", "exclude"), "utf8"), - /\.patchmill\npatchmill\.config\.json\n/u, - ); - assert.match( - stdout.join("\n"), - /Warning: Patchmill config and skills are local-only by default/u, + /patchmill\.config\.json\n\.patchmill\/\n/u, ); + assert.doesNotMatch(stdout.join("\n"), /local-only by default/u); assert.doesNotMatch(stdout.join("\n"), /Commit \.patchmill\/skills\//); assert.match( stdout.join("\n"), diff --git a/src/cli/commands/init/main.ts b/src/cli/commands/init/main.ts index ecf4c0d..953e708 100644 --- a/src/cli/commands/init/main.ts +++ b/src/cli/commands/init/main.ts @@ -5,9 +5,11 @@ import { cwd, } from "node:process"; import { createInterface } from "node:readline/promises"; +import { dirname } from "node:path"; import { pathToFileURL } from "node:url"; import { parseArgs } from "./args.ts"; import { createCommandRunner } from "../triage/command.ts"; +import type { CommandRunner } from "../triage/types.ts"; import { DEFAULT_PATCHMILL_CONFIG } from "../../../config/defaults.ts"; import { GLOBAL_PATCHMILL_SKILLS } from "../../../workflow/skills.ts"; import { createIssueHostProvider } from "../../../host/factory.ts"; @@ -23,7 +25,7 @@ import { writeInitialConfig, type InitialConfigSkills, } from "./config-writer.ts"; -import { ensurePatchmillLocalExcludeEntries } from "./local-ignore.ts"; +import { applyInitGitPolicy, selectInitGitPolicy } from "./git-policy.ts"; import { localPiAgentDir, readLocalPiDefaultModel, @@ -87,6 +89,18 @@ const DEFAULT_GLOBAL_SKILLS: InitialConfigSkills = { implementation: GLOBAL_PATCHMILL_SKILLS.implementation, }; +function stageableSkillRoots( + skills: InitialConfigSkills | undefined, +): string[] { + if (!skills) return []; + const roots = Object.values(skills).flatMap((skill) => { + if (!skill.includes("/")) return []; + const root = dirname(skill); + return root === "." ? [] : [root]; + }); + return [...new Set(roots)]; +} + const EXISTING_CONFIG_MESSAGE = "patchmill.config.json already exists.\n\nPatchmill did not overwrite it.\n\nNext:\n patchmill doctor"; @@ -142,6 +156,7 @@ export async function runInit( installProjectSkills?: ProjectSkillInstaller; validateExistingSkillDirectory?: ExistingSkillDirectoryValidator; setupLabels?: typeof ensureRequiredLabels; + commandRunner?: CommandRunner; } = {}, ): Promise { const config = parseArgs(args, repoRoot); @@ -163,7 +178,7 @@ export async function runInit( options.installProjectSkills ?? installProjectSkills )({ repoRoot: config.repoRoot }); skills = installResult.skillConfig; - skillsMessage = `Installed project-local skills:\n ${installResult.installedSkills.join("\n ")}\n\nProject-local skills are local-only by default.\n\nUsing Patchmill defaults for labels, paths, and git policy.`; + skillsMessage = `Installed project-local skills:\n ${installResult.installedSkills.join("\n ")}\n\nUsing Patchmill defaults for labels, paths, and git policy.`; } else if (config.skills.mode === "global") { skills = DEFAULT_GLOBAL_SKILLS; skillsMessage = @@ -183,17 +198,18 @@ export async function runInit( output.stdout(EXISTING_CONFIG_MESSAGE); return 1; } - const localExclude = await ensurePatchmillLocalExcludeEntries( - config.repoRoot, - ); - const localExcludeMessage = localExclude.skipped - ? `Warning: Patchmill could not update .git/info/exclude (${localExclude.skipped}).\nAdd .patchmill and patchmill.config.json to your local git excludes to keep the worktree clean.` - : localExclude.added.length > 0 - ? `Added Patchmill local files to .git/info/exclude:\n ${localExclude.added.join("\n ")}` - : "Patchmill local files were already ignored by .git/info/exclude."; - const consistencyWarning = - "Warning: Patchmill config and skills are local-only by default. For consistent Patchmill runs across local machines and CI, consider committing patchmill.config.json and .patchmill/skills/ explicitly."; const isInteractive = options.isInteractive ?? defaultStdin.isTTY; + const gitPolicy = await selectInitGitPolicy({ + isInteractive, + assumeYes: config.yes, + prompt: options.prompt ?? defaultPrompt, + }); + const gitPolicyResult = await applyInitGitPolicy({ + repoRoot: config.repoRoot, + policy: gitPolicy, + runner: options.commandRunner ?? createCommandRunner(), + skillRoots: stageableSkillRoots(result.config.skills), + }); const piAgentDir = localPiAgentDir(config.repoRoot); let currentDefault: Awaited> = undefined; @@ -271,7 +287,7 @@ export async function runInit( : ""; output.stdout( - `Created patchmill.config.json\n\nHost:\n provider: ${result.config.host.provider}\n login: ${result.config.host.login}\n\n${HOST_LOGIN_GUIDANCE}\n\n${localExcludeMessage}\n\n${consistencyWarning}\n\n${skillsMessage}\n\n${labelSetupMessage}\n\n${piSettingsMessage}${piMessage}\n\n${nextSteps(piReady)}`, + `Created patchmill.config.json\n\nHost:\n provider: ${result.config.host.provider}\n login: ${result.config.host.login}\n\n${HOST_LOGIN_GUIDANCE}\n\n${gitPolicyResult.message}\n\n${skillsMessage}\n\n${labelSetupMessage}\n\n${piSettingsMessage}${piMessage}\n\n${nextSteps(piReady)}`, ); return shouldAbortPiSetup(piSetup) ? 1 : 0; } diff --git a/src/cli/commands/init/pi-init-flow.test.ts b/src/cli/commands/init/pi-init-flow.test.ts index f837260..e8ed86c 100644 --- a/src/cli/commands/init/pi-init-flow.test.ts +++ b/src/cli/commands/init/pi-init-flow.test.ts @@ -90,6 +90,13 @@ async function skippedLabels() { }; } +function assertOnlyGitPolicyPrompt(prompts: string[]) { + assert.deepEqual( + prompts.map((prompt) => prompt.split("\n")[0]), + ["How should Patchmill files be handled by git?"], + ); +} + test("runInit uses Pi-reported ready configuration without launching a prompt selector", async () => { const repoRoot = await tempRepo(); const stdout: string[] = []; @@ -120,7 +127,7 @@ test("runInit uses Pi-reported ready configuration without launching a prompt se 0, ); - assert.deepEqual(prompts, []); + assertOnlyGitPolicyPrompt(prompts); assert.match(stdout.join("\n"), /Pi reported 1 provider\/model option/); assert.match( stdout.join("\n"), @@ -142,6 +149,7 @@ test("runInit starts Patchmill-owned interactive Pi setup when readiness is miss { stdout: (line) => stdout.push(line), stderr: () => undefined }, { isInteractive: true, + prompt: async () => "", detectPiReadiness: missingPiReadiness, resolvePiInitSetup: (setupOptions) => resolvePiInitSetup({ @@ -221,6 +229,7 @@ test("runInit persists selected model to local Pi settings and smoke-tests it", { stdout: (line) => stdout.push(line), stderr: () => undefined }, { isInteractive: true, + prompt: async () => "", setupLabels: skippedLabels, detectPiReadiness: () => readyReadiness([anthropicModel(), codexModel()]), @@ -284,6 +293,7 @@ test("runInit warns when saving the selected model to local Pi settings fails", { stdout: (line) => stdout.push(line), stderr: () => undefined }, { isInteractive: true, + prompt: async () => "", setupLabels: skippedLabels, detectPiReadiness: () => readyReadiness([anthropicModel(), codexModel()]), @@ -345,6 +355,7 @@ test("runInit warns and preserves invalid local Pi settings while using a ready { stdout: (line) => stdout.push(line), stderr: () => undefined }, { isInteractive: true, + prompt: async () => "", setupLabels: skippedLabels, detectPiReadiness: () => readyReadiness([anthropicModel(), codexModel()]), @@ -388,6 +399,7 @@ test("runInit skips model selector and local settings when no models are availab { stdout: () => undefined, stderr: () => undefined }, { isInteractive: true, + prompt: async () => "", detectPiReadiness: missingPiReadiness, resolvePiInitSetup: (setupOptions) => resolvePiInitSetup({ @@ -592,7 +604,7 @@ test("runInit aborts when the required interactive model selector is cancelled", ); assert.equal(smokeCalled, false); - assert.deepEqual(prompts, []); + assertOnlyGitPolicyPrompt(prompts); assert.match(stdout.join("\n"), /Pi model selection was cancelled/); assert.match(stdout.join("\n"), /Next:\n {2}patchmill doctor/); });