From dcab543e3cfdbcbb14eb1c292b08aca90a64191b Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 15:19:20 -0300 Subject: [PATCH 01/41] Allow pathspec commits and list staged paths - Add `listStagedPaths` via `git diff --staged --name-only -z`. - Pass optional pathspecs to `performCommit` after `--`. - Run `git -C ` so root-relative pathspecs work from a subdirectory. - Cover leftover staged files and subdirectory pathspecs in the integration test. --- src/infra/git/repo.ts | 36 ++++++++++++++++++------- test/infra/git/repo.integration.test.ts | 33 +++++++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 47ac8bc..c205971 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -1,6 +1,7 @@ export { checkIsGitRepo, getStagedDiff, + listStagedPaths, getLocalChangeContext, createAndSwitchBranch, performCommit, @@ -73,6 +74,14 @@ const getStagedDiff = (): Future => stdout.trim() ? Future.resolve(stdout) : Future.reject(new Error("No staged changes found")) ); +const listStagedPaths = (): Future => + execGitChecked(["diff", "--staged", "--name-only", "-z"], "Failed to list staged files").chain((stdout) => { + const files = stdout.split("\0").filter((p) => p.length > 0); + return files.length > 0 ? + Future.resolve(files) + : Future.reject(new Error("No staged changes found")); + }); + const NO_LOCAL_CHANGES_MESSAGE = "No local changes to infer a branch name from"; const isNoLocalChangesError = (err: unknown): err is Error => err instanceof Error && err.message === NO_LOCAL_CHANGES_MESSAGE; @@ -94,18 +103,25 @@ const getLocalChangeContext = (): Future => const createAndSwitchBranch = (name: string): Future => execGitChecked(["switch", "-c", name], `Failed to create branch '${name}'`).map(() => {}); -const performCommit = (message: string): Future => { +const getWorkTreeRoot = (): Future => + execGitChecked(["rev-parse", "--show-toplevel"], "Failed to resolve git directory").map((s) => s.trim()); + +const performCommit = (message: string, paths: readonly string[] = []): Future => { const tmpPath = join(tmpdir(), `commit-msg-${Date.now()}.txt`); - return Future.bracket( - Future.attemptP(() => writeFile(tmpPath, message, "utf-8")), - () => Future.attemptP(() => unlink(tmpPath).catch(() => {})), - () => execBin("git", ["commit", "-F", tmpPath]) - ).chain((result) => - result.either( - (failure) => Future.reject(new Error(commandFailureMessage(failure, "Commit failed"))), - ({ stdout }) => Future.resolve(formatCommitOutput(stdout)) + return getWorkTreeRoot() + .chain((root) => + Future.bracket( + Future.attemptP(() => writeFile(tmpPath, message, "utf-8")), + () => Future.attemptP(() => unlink(tmpPath).catch(() => {})), + () => execBin("git", paths.length > 0 ? ["-C", root, "commit", "-F", tmpPath, "--", ...paths] : ["-C", root, "commit", "-F", tmpPath]) + ) ) - ); + .chain((result) => + result.either( + (failure) => Future.reject(new Error(commandFailureMessage(failure, "Commit failed"))), + ({ stdout }) => Future.resolve(formatCommitOutput(stdout)) + ) + ); }; const performPush = (branch?: string, publish = false, forceWithLease = false): Future => { diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index fca41f0..7008848 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -110,4 +110,37 @@ describe("git repo integration", () => { chdir(prev); } }); + + it("performCommit with pathspecs leaves other staged files", async () => { + const { dir, run } = createTempGitRepo({ staged: true }); + writeFileSync(join(dir, "other.txt"), "other\n"); + run("add other.txt"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: one file", ["file.txt"])); + const stillStaged = run("diff --staged --name-only").trim(); + expect(stillStaged).toBe("other.txt"); + } finally { + chdir(prev); + } + }); + + it("performCommit pathspecs resolve from worktree root when cwd is a subdirectory", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + mkdirSync(join(dir, "app")); + writeFileSync(join(dir, "app", "nested.ts"), "export const n = 1;\n"); + run("add app/nested.ts"); + const prev = cwd(); + chdir(join(dir, "app")); + try { + const staged = await runFuture(repo.listStagedPaths()); + expect([...staged]).toEqual(["app/nested.ts"]); + await runFuture(repo.performCommit("feat: nested", staged)); + expect(run("diff --staged --name-only").trim()).toBe(""); + expect((await runFuture(repo.getCommitMetadata())).subject).toBe("feat: nested"); + } finally { + chdir(prev); + } + }); }); From 59297560a9bcda3fff193548e45801ffd3ce8357 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 15:19:49 -0300 Subject: [PATCH 02/41] Add split command for staged multi-commit plans - Parse and validate an LLM plan of commit groups against staged paths. - Wire `commit split` through the parser, aliases, and CLI. - Apply each group with pathspecs; keep edit, move, reorder, and push. - Cover plan parsing, router JSON, and apply/cancel in tests. --- README.md | 3 + index.ts | 5 +- src/cli/parser.ts | 4 + src/cli/split.ts | 296 +++++++++++++++++++++++++++++ src/domain/alias/alias.ts | 4 +- src/domain/commit/prompts.ts | 23 ++- src/domain/llm/router.ts | 22 ++- src/domain/split/plan.ts | 79 ++++++++ test/cli/parser.test.ts | 1 + test/cli/split.test.ts | 94 +++++++++ test/domain/commit/prompts.test.ts | 12 +- test/domain/llm/router.test.ts | 22 ++- test/domain/split/plan.test.ts | 71 +++++++ 13 files changed, 629 insertions(+), 7 deletions(-) create mode 100644 src/cli/split.ts create mode 100644 src/domain/split/plan.ts create mode 100644 test/cli/split.test.ts create mode 100644 test/domain/split/plan.test.ts diff --git a/README.md b/README.md index 9d67899..b093260 100644 --- a/README.md +++ b/README.md @@ -160,6 +160,8 @@ Or explicitly: commit generate ``` +`commit split` for a multi-commit plan. + ### System Checks Verify your installation, environment, and configuration: @@ -198,6 +200,7 @@ commit --help | ------------------------ | ------------------------------------------------- | | `commit` | Generate a commit message (default) | | `commit generate` | Generate a commit message | +| `commit split` | Split staged changes into multiple commits | | `commit setup` | Configure authentication and conventions | | `commit login` | Alias for setup — re-authenticate | | `commit doctor` | Check installation and environment | diff --git a/index.ts b/index.ts index f2bfe8a..faec76a 100755 --- a/index.ts +++ b/index.ts @@ -1,4 +1,5 @@ import { Commit } from "@/cli/commit"; +import { Split } from "@/cli/split"; import { Branch } from "@/cli/branch"; import { Setup } from "@/cli/setup"; import { Doctor } from "@/cli/doctor"; @@ -13,7 +14,7 @@ import { checkUpdate } from "@/cli/update"; import color from "picocolors"; -const NOTIFIER_COMMANDS = new Set(["generate", "setup", "doctor", "model", "effort", "branch", "alias"]); +const NOTIFIER_COMMANDS = new Set(["generate", "split", "setup", "doctor", "model", "effort", "branch", "alias"]); const main = () => { const args = process.argv.slice(2); @@ -29,6 +30,8 @@ const main = () => { switch (command.type) { case "generate": return Commit.create().chain((c) => c.run()); + case "split": + return Split.create().chain((s) => s.run()); case "setup": return Setup.create().chain((s) => s.run()); case "doctor": diff --git a/src/cli/parser.ts b/src/cli/parser.ts index 2e80b31..c0dafb9 100644 --- a/src/cli/parser.ts +++ b/src/cli/parser.ts @@ -10,6 +10,7 @@ type AliasAction = { type: "hub" } | { type: "list" } | { type: "add"; name: str type CliCommand = | { type: "generate" } + | { type: "split" } | { type: "setup" } | { type: "doctor" } | { type: "model" } @@ -53,6 +54,8 @@ const cliCommandDecoder: D.Decoder = D.array(D.string).chain((args) switch (cmd) { case "generate": return D.succeed({ type: "generate" }); + case "split": + return D.succeed({ type: "split" }); case "setup": case "login": return D.succeed({ type: "setup" }); @@ -89,6 +92,7 @@ Usage: commit-tools [command] Commands: generate (default) Generate a commit message + split Split staged changes into multiple commits branch Suggest branch names from local changes and create one new-branch Alias for branch setup Configure authentication and conventions diff --git a/src/cli/split.ts b/src/cli/split.ts new file mode 100644 index 0000000..6356817 --- /dev/null +++ b/src/cli/split.ts @@ -0,0 +1,296 @@ +export { Split }; + +import * as p from "@clack/prompts"; +import * as pr from "@/infra/github/pr"; +import * as repo from "@/infra/git/repo"; + +import { Future } from "@/libs/future"; +import { loadConfig } from "@/infra/storage/config"; +import { Setup } from "@/cli/setup"; +import { Commit } from "@/cli/commit"; +import { type Config, type ProviderConfig } from "@/domain/config/config"; +import { resolveProvider } from "@/domain/llm/auth-resolver"; +import { generateSplitPlan, type LlmRequestMetadata, type SplitPlanContent } from "@/domain/llm/router"; +import { type SplitPlan } from "@/domain/split/plan"; +import { Just, type Maybe } from "@/libs/maybe"; +import { loading } from "@/infra/ui/spinner"; +import { renderCommitNote, renderPushNote } from "@/infra/ui/push-note"; +import { absurd } from "@/libs/types"; + +import color from "picocolors"; + +const SPLIT_ACTIONS = ["apply_push", "apply", "edit", "move", "reorder", "regenerate", "cancel"] as const; +type SplitAction = (typeof SPLIT_ACTIONS)[number]; + +const formatPlanNote = (plan: SplitPlan, stagedCount: number): string => { + const groups = plan.commits.map((commit, index) => `${index + 1}. ${commit.message}\n ${commit.files.join(" ")}`).join("\n\n"); + return `${plan.commits.length} commits · ${stagedCount} staged files\n\n${groups}`; +}; + +const commitOptions = (plan: SplitPlan): { value: number; label: string }[] => + plan.commits.map((commit, index) => ({ value: index, label: `${index + 1}. ${commit.message}` })); + +const withEditedMessage = (plan: SplitPlan, index: number, message: string): SplitPlan => ({ + commits: plan.commits.map((commit, i) => (i === index ? { message, files: commit.files } : commit)) +}); + +const withMovedFile = (plan: SplitPlan, file: string, destIndex: number): SplitPlan => { + const sourceIndex = plan.commits.findIndex((commit) => commit.files.includes(file)); + if (sourceIndex === -1 || sourceIndex === destIndex) { + return plan; + } + const moved = plan.commits.map((commit, i) => { + if (i === sourceIndex) { + return { message: commit.message, files: commit.files.filter((path) => path !== file) }; + } + if (i === destIndex) { + return { message: commit.message, files: [...commit.files, file] }; + } + return commit; + }); + return { commits: moved.filter((commit) => commit.files.length > 0) }; +}; + +const withReorderedCommit = (plan: SplitPlan, fromIndex: number, toIndex: number): SplitPlan => { + if (fromIndex === toIndex) { + return plan; + } + const commits = [...plan.commits]; + const [item] = commits.splice(fromIndex, 1); + if (item === undefined) { + return plan; + } + commits.splice(toIndex, 0, item); + return { commits }; +}; + +class Split { + private constructor( + private readonly config: Config, + private readonly providerConfig: ProviderConfig + ) {} + + static create(): Future { + return loadConfig() + .chainRej((): Future => { + p.log.warn(color.yellow("No configuration found. Let's set you up first.")); + return Setup.create() + .chain((s) => s.run()) + .chain(() => loadConfig()); + }) + .chain((config) => resolveProvider(config).map((ai) => new Split(config, ai))); + } + + run(): Future { + return repo + .checkIsGitRepo() + .chain(() => Future.concurrently({ diff: repo.getStagedDiff(), files: repo.listStagedPaths() })) + .chain(({ diff, files }) => this.generate(diff, files).chain((content) => this.interact(diff, files, content.plan, content.metadata))) + .mapRej((e) => { + p.log.error(color.red(e.message)); + return e; + }); + } + + private generate(diff: string, files: readonly string[]): Future { + return loading( + "Generating split plan...", + "Split plan generated!", + generateSplitPlan(this.providerConfig, diff, files, this.config.commit_convention, this.config.custom_template) + ); + } + + private interact(diff: string, files: readonly string[], plan: SplitPlan, meta: LlmRequestMetadata): Future { + return this.promptAction(plan, files.length).chain((action) => { + switch (action) { + case "apply_push": + return this.apply(plan, meta, true); + case "apply": + return this.apply(plan, meta, false); + case "edit": + return this.editMessage(plan).chain((next) => this.interact(diff, files, next, meta)); + case "move": + return this.moveFile(plan).chain((next) => this.interact(diff, files, next, meta)); + case "reorder": + return this.reorder(plan).chain((next) => this.interact(diff, files, next, meta)); + case "regenerate": + return this.generate(diff, files).chain((c) => this.interact(diff, files, c.plan, c.metadata)); + case "cancel": + return Future.resolve(undefined); + default: + return absurd(action, "SplitAction"); + } + }); + } + + private promptAction(plan: SplitPlan, stagedCount: number): Future { + return Future.attemptP(async () => { + p.note(formatPlanNote(plan, stagedCount), "Split Plan"); + + const action = await p.select({ + message: "What would you like to do?", + options: [ + { value: "apply_push", label: "Apply & Push" }, + { value: "apply", label: "Apply" }, + { value: "edit", label: "Edit message" }, + { value: "move", label: "Move file" }, + { value: "reorder", label: "Reorder" }, + { value: "regenerate", label: "Regenerate" }, + { value: "cancel", label: "Cancel" } + ] + }); + + if (p.isCancel(action) || action === "cancel") { + p.outro("Operation cancelled."); + return "cancel"; + } + + return action; + }); + } + + private editMessage(plan: SplitPlan): Future { + return Future.attemptP(async () => { + const picked = await p.select({ + message: "Which commit?", + options: commitOptions(plan) + }); + if (p.isCancel(picked)) { + return plan; + } + const current = plan.commits[picked]; + if (current === undefined) { + return plan; + } + const next = await p.text({ + message: "New message", + initialValue: current.message + }); + if (p.isCancel(next)) { + return plan; + } + const message = next.trim(); + return message.length === 0 ? plan : withEditedMessage(plan, picked, message); + }); + } + + private moveFile(plan: SplitPlan): Future { + return Future.attemptP(async () => { + const file = await p.select({ + message: "Move which file?", + options: plan.commits.flatMap((commit) => commit.files.map((path) => ({ value: path, label: path }))) + }); + if (p.isCancel(file)) { + return plan; + } + const dest = await p.select({ + message: "Move to which commit?", + options: commitOptions(plan) + }); + if (p.isCancel(dest)) { + return plan; + } + return withMovedFile(plan, file, dest); + }); + } + + private reorder(plan: SplitPlan): Future { + return Future.attemptP(async () => { + const from = await p.select({ + message: "Which commit?", + options: commitOptions(plan) + }); + if (p.isCancel(from)) { + return plan; + } + const position = await p.select({ + message: "New position", + options: plan.commits.map((_, index) => ({ value: index + 1, label: String(index + 1) })) + }); + if (p.isCancel(position)) { + return plan; + } + return withReorderedCommit(plan, from, position - 1); + }); + } + + private apply(plan: SplitPlan, meta: LlmRequestMetadata, shouldPush: boolean): Future { + return Future.traverse( + (group) => + repo.performCommit(group.message, group.files).map((stats) => { + process.stdout.write(stats); + }), + [...plan.commits] + ).chain(() => + shouldPush ? + this.pushAfterCommit(Just(meta)).map(() => { + p.outro(color.green("Done!")); + }) + : repo.findCommitMetadata().map((commit) => { + renderCommitNote({ commit, request: Just(meta) }); + p.outro(color.green("Committed successfully!")); + }) + ); + } + + private push(request: Maybe, branch?: string, publish = false, forceWithLease = false): Future { + const startMsg = + forceWithLease ? "Force pushing with lease..." + : publish ? `Publishing '${branch}'...` + : "Pushing..."; + + const endMsg = + forceWithLease ? "Force pushed successfully!" + : publish ? "Published successfully!" + : "Pushed successfully!"; + + return loading(startMsg, endMsg, repo.performPush(branch, publish, forceWithLease)).chain((result) => + Future.concurrently< + Error, + { + commit: Maybe; + localBranch: Maybe; + baseBranch: Maybe; + remoteUrl: Maybe; + pr: pr.PrLookup; + } + >({ + commit: repo.findCommitMetadata(), + localBranch: repo.findCurrentBranch(), + baseBranch: repo.findBaseBranch(), + remoteUrl: repo.findTrackingRemoteUrl(), + pr: pr.getOpenPullRequest() + }).map((parts) => renderPushNote({ ...parts, range: result.range, request })) + ); + } + + private pushAfterCommit(request: Maybe): Future { + return repo + .hasUpstream() + .chain((exists) => + exists ? + this.push(request).chainRej((err) => (Commit.isNonFastForwardError(err) ? this.promptForceWithLease(request) : Future.reject(err))) + : this.promptPublishBranch(request) + ); + } + + private promptPublishBranch(request: Maybe): Future { + return repo.getCurrentBranch().chain((branch) => + Future.attemptP(async () => { + const publish = await p.confirm({ + message: `Branch '${branch}' has no upstream. Publish to origin?` + }); + return !(p.isCancel(publish) || !publish); + }).chain((shouldPublish) => (shouldPublish ? this.push(request, branch, true) : Future.resolve(undefined))) + ); + } + + private promptForceWithLease(request: Maybe): Future { + return Future.attemptP(async () => { + const force = await p.confirm({ + message: "Push was rejected (branch is behind remote). Force push with lease?" + }); + return !(p.isCancel(force) || !force); + }).chain((shouldForce) => (shouldForce ? this.push(request, undefined, false, true) : Future.resolve(undefined))); + } +} diff --git a/src/domain/alias/alias.ts b/src/domain/alias/alias.ts index c0fba87..4f30875 100644 --- a/src/domain/alias/alias.ts +++ b/src/domain/alias/alias.ts @@ -6,7 +6,7 @@ import { Failure, Success, type Result } from "@/libs/result"; import { fromOptional, Just, type Maybe } from "@/libs/maybe"; import { absurd } from "@/libs/types"; -const ALIAS_TARGETS = ["generate", "branch", "setup", "doctor", "model", "effort", "update"] as const; +const ALIAS_TARGETS = ["generate", "split", "branch", "setup", "doctor", "model", "effort", "update"] as const; type AliasTarget = (typeof ALIAS_TARGETS)[number]; const NAME_PATTERN = /^[a-zA-Z][a-zA-Z0-9_-]{0,31}$/; @@ -57,6 +57,8 @@ const describeTarget = (target: AliasTarget): string => { switch (target) { case "generate": return "Generate a commit message"; + case "split": + return "Split staged changes into multiple commits"; case "branch": return "Suggest branch names and create one"; case "setup": diff --git a/src/domain/commit/prompts.ts b/src/domain/commit/prompts.ts index 4b988be..a5aa286 100644 --- a/src/domain/commit/prompts.ts +++ b/src/domain/commit/prompts.ts @@ -1,4 +1,4 @@ -export { getPrompt, getRefinePrompt, getBranchNamePrompt }; +export { getPrompt, getRefinePrompt, getBranchNamePrompt, getSplitPrompt }; import { CommitConvention } from "@/domain/config/config"; import { Just, Nothing, type Maybe } from "@/libs/maybe"; @@ -275,6 +275,27 @@ function promptCustom(gitDiff: string, template: Maybe): string { } } +function getSplitPrompt(diff: string, files: readonly string[], convention: CommitConvention, customTemplate: Maybe = Nothing()): string { + return ` + ${getPrompt(diff, convention, customTemplate)} + + + ${files.join("\n")} + + + + Return ONE JSON object. First character "{", last "}". + {"commits":[{"message":"","files":["",...]}]} + + + - Use only paths from . Exact strings. + - Every staged path in exactly one commit. + - Prefer 2+ commits when files have distinct concerns; 1 commit is allowed if they are one change. + - Each message follows the active convention (SMALL/MEDIUM/LARGE shape). + + `; +} + function getBranchNamePrompt(context: string): string { return ` diff --git a/src/domain/llm/router.ts b/src/domain/llm/router.ts index 0bbafb2..364f147 100644 --- a/src/domain/llm/router.ts +++ b/src/domain/llm/router.ts @@ -9,7 +9,9 @@ export { type BranchSuggestion, generateCommitMessage, refineCommitMessage, - generateBranchNameSuggestions + generateBranchNameSuggestions, + generateSplitPlan, + type SplitPlanContent }; import { Future } from "@/libs/future"; @@ -19,8 +21,9 @@ import { generateContentWithGemini } from "@/infra/llm/gemini"; import { generateContentWithOpenAI } from "@/infra/llm/openai"; import { generateContentWithAnthropic } from "@/infra/llm/anthropic"; import { generateContentWithXai } from "@/infra/llm/xai"; -import { getPrompt, getRefinePrompt, getBranchNamePrompt } from "@/domain/commit/prompts"; +import { getPrompt, getRefinePrompt, getBranchNamePrompt, getSplitPrompt } from "@/domain/commit/prompts"; import { parseAndValidateBranchSuggestions, type BranchSuggestion } from "@/domain/branch/suggestions"; +import { parseAndValidateSplitPlan, type SplitPlan } from "@/domain/split/plan"; import { withTransientRetry } from "@/domain/llm/retry"; import { Maybe, Nothing } from "@/libs/maybe"; @@ -57,6 +60,8 @@ type BranchNameSuggestions = { readonly metadata: LlmRequestMetadata; }; +type SplitPlanContent = { readonly plan: SplitPlan; readonly metadata: LlmRequestMetadata }; + type ProviderGeneratedContent = { readonly text: string; readonly tokens: Maybe; @@ -132,3 +137,16 @@ const generateBranchNameSuggestions = (config: ProviderConfig, context: string): })) ) ); + +const generateSplitPlan = ( + config: ProviderConfig, + diff: string, + files: readonly string[], + convention: CommitConvention, + customTemplate: Maybe +): Future => + withTransientRetry(() => + generateContent(config, { prompt: getSplitPrompt(diff, files, convention, customTemplate) }).chain((gc) => + resultToFuture(parseAndValidateSplitPlan(gc.text, files)).map((plan) => ({ plan, metadata: gc.metadata })) + ) + ); diff --git a/src/domain/split/plan.ts b/src/domain/split/plan.ts new file mode 100644 index 0000000..45f352b --- /dev/null +++ b/src/domain/split/plan.ts @@ -0,0 +1,79 @@ +export { parseAndValidateSplitPlan, type SplitCommit, type SplitPlan }; + +import * as D from "@/libs/json/decoder"; +import { Failure, Success, type Result } from "@/libs/result"; + +type SplitCommit = { readonly message: string; readonly files: readonly string[] }; +type SplitPlan = { readonly commits: readonly SplitCommit[] }; + +const REMAINING_STAGED_MESSAGE = "Commit remaining staged changes"; + +const nonEmptyString = (label: string): D.Decoder => + D.string.chain((s) => { + const t = s.trim(); + return t.length === 0 ? D.fail(`${label} must be non-empty`) : D.succeed(t); + }); + +const nonEmptyFiles: D.Decoder = D.array(nonEmptyString("file")).chain((xs) => + xs.length === 0 ? D.fail("files must be non-empty") : D.succeed(xs) +); + +const splitCommitDecoder: D.Decoder = D.object({ + message: nonEmptyString("message"), + files: nonEmptyFiles +}); + +const splitPlanDecoder: D.Decoder = D.object({ + commits: D.array(splitCommitDecoder).chain((xs) => (xs.length === 0 ? D.fail("expected at least 1 commit") : D.succeed(xs))) +}); + +const stripOptionalJsonFence = (s: string): string => { + const t = s.trim(); + if (!t.startsWith("```")) { + return t; + } + const firstNl = t.indexOf("\n"); + const body = firstNl === -1 ? "" : t.slice(firstNl + 1); + const close = body.indexOf("```"); + if (close === -1) { + return body.trim(); + } + return body.slice(0, close).trim(); +}; + +const parseSplitPlan = (raw: string): Result => { + const trimmed = stripOptionalJsonFence(raw.trim()); + let json: unknown; + try { + json = JSON.parse(trimmed); + } catch { + return Failure(new Error("Split plan: invalid JSON")); + } + return D.decode(json, splitPlanDecoder).mapFailure((msg) => new Error(`Split plan: ${msg}`)); +}; + +const validateSplitPlan = (plan: SplitPlan, stagedFiles: readonly string[]): Result => { + const staged = new Set(stagedFiles); + const seen = new Set(); + for (const commit of plan.commits) { + for (const file of commit.files) { + if (seen.has(file)) { + return Failure(new Error(`Split plan: duplicate path: ${file}`)); + } + if (!staged.has(file)) { + return Failure(new Error(`Split plan: unknown path: ${file}`)); + } + seen.add(file); + } + } + const leftover = stagedFiles.filter((file) => !seen.has(file)); + if (leftover.length === 0) { + return Success(plan); + } + return Success({ + commits: [...plan.commits, { message: REMAINING_STAGED_MESSAGE, files: leftover }] + }); +}; + +const parseAndValidateSplitPlan = (raw: string, stagedFiles: readonly string[]): Result => + parseSplitPlan(raw).chain((plan) => validateSplitPlan(plan, stagedFiles)); diff --git a/test/cli/parser.test.ts b/test/cli/parser.test.ts index b6a4992..61325c0 100644 --- a/test/cli/parser.test.ts +++ b/test/cli/parser.test.ts @@ -6,6 +6,7 @@ import { Failure, Success } from "@/libs/result"; describe("parseArgs", () => { it.each([ [["generate"], "generate"], + [["split"], "split"], [["branch"], "branch"], [["new-branch"], "branch"], [["setup"], "setup"], diff --git a/test/cli/split.test.ts b/test/cli/split.test.ts new file mode 100644 index 0000000..54c13c9 --- /dev/null +++ b/test/cli/split.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; + +vi.mock("@/infra/env", () => ({ + environment: { GOOGLE_CLIENT_ID: "test", GOOGLE_CLIENT_SECRET: "test" } +})); + +import { Split } from "@/cli/split"; +import { Future } from "@/libs/future"; +import { Nothing, Just } from "@/libs/maybe"; +import { runFuture } from "@test/helpers/run-future"; +import * as s from "@/libs/json/schema"; +import { Config } from "@/domain/config/config"; + +type ConfigValue = s.Infer; + +vi.mock("@/infra/storage/config", () => ({ + loadConfig: vi.fn() +})); +vi.mock("@/domain/llm/auth-resolver", () => ({ + resolveProvider: vi.fn((c: ConfigValue) => Future.resolve(c.ai)) +})); +vi.mock("@/infra/git/repo", () => ({ + checkIsGitRepo: vi.fn(() => Future.resolve(undefined)), + getStagedDiff: vi.fn(() => Future.resolve("staged diff")), + listStagedPaths: vi.fn(() => Future.resolve(["a.ts", "b.ts"])), + performCommit: vi.fn(() => Future.resolve("\n 1 file changed\n")), + findCommitMetadata: vi.fn() +})); +vi.mock("@/domain/llm/router", () => ({ + generateSplitPlan: vi.fn(() => + Future.resolve({ + plan: { + commits: [ + { message: "msg one", files: ["a.ts"] }, + { message: "msg two", files: ["b.ts"] } + ] + }, + metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } + }) + ) +})); +vi.mock("@clack/prompts", () => ({ + note: vi.fn(), + select: vi.fn(async () => "apply"), + text: vi.fn(), + confirm: vi.fn(), + isCancel: vi.fn(() => false), + outro: vi.fn(), + log: { warn: vi.fn(), error: vi.fn() } +})); +vi.mock("@/infra/ui/push-note", () => ({ + renderCommitNote: vi.fn(), + renderPushNote: vi.fn() +})); +vi.mock("@/infra/ui/spinner", () => ({ + loading: vi.fn((_a: string, _b: string, f: Future) => f as Future) +})); + +const config = (): ConfigValue => ({ + commit_convention: "conventional", + custom_template: Nothing(), + ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } +}); + +describe("Split.run", () => { + beforeEach(async () => { + vi.clearAllMocks(); + const storage = await import("@/infra/storage/config"); + vi.mocked(storage.loadConfig).mockReturnValue(Future.resolve(config())); + const repo = await import("@/infra/git/repo"); + vi.mocked(repo.findCommitMetadata).mockReturnValue( + Future.resolve(Just({ hash: "h", short: "h", subject: "msg two", authorName: "t", authorEmail: "t@t.com", date: new Date() })) + ); + const prompts = await import("@clack/prompts"); + vi.mocked(prompts.select).mockResolvedValue("apply"); + vi.mocked(prompts.isCancel).mockReturnValue(false); + }); + + it("applies each commit group with pathspecs when user selects apply", async () => { + await runFuture(Split.create().chain((s) => s.run())); + const repo = await import("@/infra/git/repo"); + expect(repo.performCommit).toHaveBeenNthCalledWith(1, "msg one", ["a.ts"]); + expect(repo.performCommit).toHaveBeenNthCalledWith(2, "msg two", ["b.ts"]); + }); + + it("does not commit when user selects cancel", async () => { + const prompts = await import("@clack/prompts"); + vi.mocked(prompts.select).mockResolvedValue("cancel"); + + await runFuture(Split.create().chain((s) => s.run())); + const repo = await import("@/infra/git/repo"); + expect(repo.performCommit).not.toHaveBeenCalled(); + }); +}); diff --git a/test/domain/commit/prompts.test.ts b/test/domain/commit/prompts.test.ts index ab3eaa8..7e1f9e1 100644 --- a/test/domain/commit/prompts.test.ts +++ b/test/domain/commit/prompts.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { getPrompt, getRefinePrompt } from "@/domain/commit/prompts"; +import { getPrompt, getRefinePrompt, getSplitPrompt } from "@/domain/commit/prompts"; import { Just, Nothing } from "@/libs/maybe"; const DIFF = "diff --git a/foo.ts b/foo.ts\n+console.log(1)"; @@ -30,6 +30,16 @@ describe("getPrompt", () => { }); }); +describe("getSplitPrompt", () => { + it("embeds diff, file list, and commits", () => { + const prompt = getSplitPrompt(DIFF, ["foo.ts", "bar.ts"], "conventional"); + expect(prompt).toContain(DIFF); + expect(prompt).toContain("foo.ts"); + expect(prompt).toContain("bar.ts"); + expect(prompt).toContain("commits"); + }); +}); + describe("getRefinePrompt", () => { it("wraps diff, current message, and adjustment", () => { const { prompt, systemInstruction } = getRefinePrompt({ diff --git a/test/domain/llm/router.test.ts b/test/domain/llm/router.test.ts index a004326..8e472a8 100644 --- a/test/domain/llm/router.test.ts +++ b/test/domain/llm/router.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; -import { generateCommitMessage, refineCommitMessage } from "@/domain/llm/router"; +import { generateCommitMessage, refineCommitMessage, generateSplitPlan } from "@/domain/llm/router"; import { Future } from "@/libs/future"; import { Just, Nothing } from "@/libs/maybe"; import { runFuture } from "@test/helpers/run-future"; @@ -54,3 +54,23 @@ describe("refineCommitMessage", () => { expect(result.metadata.model.effort).toBe("provider default"); }); }); + +describe("generateSplitPlan", () => { + it("parses provider JSON into commits", async () => { + const { generateContentWithOpenAI } = await import("@/infra/llm/openai"); + const json = JSON.stringify({ + commits: [ + { message: "feat: a", files: ["a.ts"] }, + { message: "feat: b", files: ["b.ts"] } + ] + }); + vi.mocked(generateContentWithOpenAI).mockReturnValue(Future.resolve({ text: json, tokens: Nothing(), effectiveEffort: Nothing() })); + + const result = await runFuture(generateSplitPlan(mockProvider("openai"), "diff", ["a.ts", "b.ts"], "conventional", Nothing())); + expect(result.plan.commits).toEqual([ + { message: "feat: a", files: ["a.ts"] }, + { message: "feat: b", files: ["b.ts"] } + ]); + expect(result.metadata.model.provider).toBe("openai"); + }); +}); diff --git a/test/domain/split/plan.test.ts b/test/domain/split/plan.test.ts new file mode 100644 index 0000000..877b871 --- /dev/null +++ b/test/domain/split/plan.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { Failure, Success } from "@/libs/result"; +import { parseAndValidateSplitPlan } from "@/domain/split/plan"; + +const staged = ["a.ts", "b.ts", "c.ts"] as const; + +const threeCommits = JSON.stringify({ + commits: [ + { message: "feat: a", files: ["a.ts"] }, + { message: "feat: b", files: ["b.ts"] }, + { message: "feat: c", files: ["c.ts"] } + ] +}); + +describe("parseAndValidateSplitPlan", () => { + it("parses valid 3-commit JSON", () => { + const r = parseAndValidateSplitPlan(threeCommits, staged); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value.commits).toEqual([ + { message: "feat: a", files: ["a.ts"] }, + { message: "feat: b", files: ["b.ts"] }, + { message: "feat: c", files: ["c.ts"] } + ]); + } + }); + + it("parses fenced JSON", () => { + const r = parseAndValidateSplitPlan("```json\n" + threeCommits + "\n```", staged); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value.commits.map((c) => c.message)).toEqual(["feat: a", "feat: b", "feat: c"]); + } + }); + + it("rejects empty commits", () => { + const r = parseAndValidateSplitPlan('{"commits":[]}', staged); + expect(r instanceof Failure).toBe(true); + }); + + it("rejects empty message", () => { + const r = parseAndValidateSplitPlan('{"commits":[{"message":"","files":["a.ts"]}]}', staged); + expect(r instanceof Failure).toBe(true); + }); + + it("rejects empty files", () => { + const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":[]}]}', staged); + expect(r instanceof Failure).toBe(true); + }); + + it("rejects duplicate paths", () => { + const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":["a.ts"]},{"message":"feat: again","files":["a.ts"]}]}', staged); + expect(r instanceof Failure).toBe(true); + }); + + it("rejects unknown path", () => { + const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: x","files":["missing.ts"]}]}', staged); + expect(r instanceof Failure).toBe(true); + }); + + it("appends leftover staged paths", () => { + const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":["a.ts"]}]}', staged); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value.commits).toEqual([ + { message: "feat: a", files: ["a.ts"] }, + { message: "Commit remaining staged changes", files: ["b.ts", "c.ts"] } + ]); + } + }); +}); From bbb85d0a1aa0c19f8475a24f905be41116e87a46 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 03/41] Add optional split_commits config defaulting to false - Persist `split_commits` on the config schema with a false default. - Update config fixtures and storage tests for the new field. --- src/domain/config/config.ts | 3 ++- test/cli/branch.test.ts | 1 + test/cli/effort.test.ts | 2 ++ test/cli/model.test.ts | 1 + test/domain/config/config.test.ts | 9 +++++++++ test/domain/llm/auth-resolver.test.ts | 2 ++ test/infra/storage/config.test.ts | 2 ++ 7 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/domain/config/config.ts b/src/domain/config/config.ts index 429d7c3..b7b9bcb 100644 --- a/src/domain/config/config.ts +++ b/src/domain/config/config.ts @@ -138,7 +138,8 @@ const resolveAuthMethod = (ai: ProviderConfig, auth_method: ProviderConfig["auth const Config = s.object({ ai: schema_ProviderConfig, commit_convention: s.stringEnum([...COMMIT_CONVENTIONS]), - custom_template: s.optionalMaybe(s.string) + custom_template: s.optionalMaybe(s.string), + split_commits: s.optionalDefault(false, s.boolean) }); type Config = s.Infer; diff --git a/test/cli/branch.test.ts b/test/cli/branch.test.ts index 1353288..6d89371 100644 --- a/test/cli/branch.test.ts +++ b/test/cli/branch.test.ts @@ -65,6 +65,7 @@ vi.mock("@/infra/ui/spinner", () => ({ const config = (): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } }); diff --git a/test/cli/effort.test.ts b/test/cli/effort.test.ts index 9ffee0d..e6260d4 100644 --- a/test/cli/effort.test.ts +++ b/test/cli/effort.test.ts @@ -38,6 +38,7 @@ describe("EffortCommand", () => { Future.resolve({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } } satisfies ConfigValue) ); @@ -53,6 +54,7 @@ describe("EffortCommand", () => { const config: ConfigValue = { commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-5.6-sol", diff --git a/test/cli/model.test.ts b/test/cli/model.test.ts index 77ee9b0..17464fd 100644 --- a/test/cli/model.test.ts +++ b/test/cli/model.test.ts @@ -11,6 +11,7 @@ type ConfigValue = s.Infer; const config = (): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "old", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } }); diff --git a/test/domain/config/config.test.ts b/test/domain/config/config.test.ts index 844f161..108cabb 100644 --- a/test/domain/config/config.test.ts +++ b/test/domain/config/config.test.ts @@ -9,6 +9,7 @@ type ConfigValue = s.Infer; const sampleConfig = (): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-4.1-mini", @@ -80,4 +81,12 @@ describe("Config schema", () => { expect(s.decode(Config, encoded).isSuccess()).toBe(true); }); + + it("defaults missing split_commits to false", () => { + const encoded = s.encode(Config, sampleConfig()) as Record; + delete encoded["split_commits"]; + const decoded = s.decode(Config, encoded); + expect(decoded.isSuccess()).toBe(true); + if (decoded instanceof Success) expect(decoded.value.split_commits).toBe(false); + }); }); diff --git a/test/domain/llm/auth-resolver.test.ts b/test/domain/llm/auth-resolver.test.ts index 79d8225..e8f9e04 100644 --- a/test/domain/llm/auth-resolver.test.ts +++ b/test/domain/llm/auth-resolver.test.ts @@ -19,6 +19,7 @@ type ConfigValue = s.Infer; const googleConfig = (): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "gemini", model: "gemini-2.0", @@ -58,6 +59,7 @@ describe("resolveProvider", () => { const config: ConfigValue = { commit_convention: "imperative", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk-x" } } }; const ai = await runFuture(resolveProvider(config)); diff --git a/test/infra/storage/config.test.ts b/test/infra/storage/config.test.ts index 7f01b03..6bc2dae 100644 --- a/test/infra/storage/config.test.ts +++ b/test/infra/storage/config.test.ts @@ -11,6 +11,7 @@ type ConfigValue = s.Infer; const sampleConfig = (): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-4.1-mini", @@ -53,6 +54,7 @@ describe("config storage", () => { saveConfig({ commit_convention: "imperative", custom_template: Just("tpl"), + split_commits: false, ai: { ...sampleConfig().ai, auth_method: { type: "openai_oauth", content: staleTokens() } } }) ); From a9a74f8da53e5c26862b3c79fc824d8adc4d767a Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 04/41] Require should_split on parsed split plans - Decode `should_split` and keep it through plan edits. - Extract JSON from prose and fold leftovers when not splitting. --- src/domain/split/plan.ts | 27 ++++++++++++++++++++--- test/domain/split/plan.test.ts | 40 +++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/domain/split/plan.ts b/src/domain/split/plan.ts index 45f352b..8b6cb07 100644 --- a/src/domain/split/plan.ts +++ b/src/domain/split/plan.ts @@ -4,7 +4,7 @@ import * as D from "@/libs/json/decoder"; import { Failure, Success, type Result } from "@/libs/result"; type SplitCommit = { readonly message: string; readonly files: readonly string[] }; -type SplitPlan = { readonly commits: readonly SplitCommit[] }; +type SplitPlan = { readonly commits: readonly SplitCommit[]; readonly shouldSplit: boolean }; const REMAINING_STAGED_MESSAGE = "Commit remaining staged changes"; @@ -24,8 +24,9 @@ const splitCommitDecoder: D.Decoder = D.object({ }); const splitPlanDecoder: D.Decoder = D.object({ + should_split: D.boolean, commits: D.array(splitCommitDecoder).chain((xs) => (xs.length === 0 ? D.fail("expected at least 1 commit") : D.succeed(xs))) -}); +}).map(({ should_split, commits }) => ({ shouldSplit: should_split, commits })); const stripOptionalJsonFence = (s: string): string => { const t = s.trim(); @@ -41,8 +42,17 @@ const stripOptionalJsonFence = (s: string): string => { return body.slice(0, close).trim(); }; +const extractJsonObject = (s: string): string => { + const start = s.indexOf("{"); + const end = s.lastIndexOf("}"); + if (start === -1 || end <= start) { + return s; + } + return s.slice(start, end + 1); +}; + const parseSplitPlan = (raw: string): Result => { - const trimmed = stripOptionalJsonFence(raw.trim()); + const trimmed = extractJsonObject(stripOptionalJsonFence(raw.trim())); let json: unknown; try { json = JSON.parse(trimmed); @@ -70,7 +80,18 @@ const validateSplitPlan = (plan: SplitPlan, stagedFiles: readonly string[]): Res if (leftover.length === 0) { return Success(plan); } + if (!plan.shouldSplit) { + const [first, ...rest] = plan.commits; + if (first === undefined) { + return Failure(new Error("Split plan: expected at least 1 commit")); + } + return Success({ + shouldSplit: false, + commits: [{ message: first.message, files: [...first.files, ...leftover] }, ...rest] + }); + } return Success({ + shouldSplit: true, commits: [...plan.commits, { message: REMAINING_STAGED_MESSAGE, files: leftover }] }); }; diff --git a/test/domain/split/plan.test.ts b/test/domain/split/plan.test.ts index 877b871..1e8ecf9 100644 --- a/test/domain/split/plan.test.ts +++ b/test/domain/split/plan.test.ts @@ -5,6 +5,7 @@ import { parseAndValidateSplitPlan } from "@/domain/split/plan"; const staged = ["a.ts", "b.ts", "c.ts"] as const; const threeCommits = JSON.stringify({ + should_split: true, commits: [ { message: "feat: a", files: ["a.ts"] }, { message: "feat: b", files: ["b.ts"] }, @@ -33,33 +34,46 @@ describe("parseAndValidateSplitPlan", () => { } }); + it("parses JSON wrapped in prose", () => { + const r = parseAndValidateSplitPlan("Here is the plan:\n" + threeCommits + "\nDone.", staged); + expect(r instanceof Success).toBe(true); + }); + + it("rejects invalid JSON", () => { + const r = parseAndValidateSplitPlan("feat: add foo", staged); + expect(r instanceof Failure).toBe(true); + }); + it("rejects empty commits", () => { - const r = parseAndValidateSplitPlan('{"commits":[]}', staged); + const r = parseAndValidateSplitPlan('{"should_split":true,"commits":[]}', staged); expect(r instanceof Failure).toBe(true); }); it("rejects empty message", () => { - const r = parseAndValidateSplitPlan('{"commits":[{"message":"","files":["a.ts"]}]}', staged); + const r = parseAndValidateSplitPlan('{"should_split":true,"commits":[{"message":"","files":["a.ts"]}]}', staged); expect(r instanceof Failure).toBe(true); }); it("rejects empty files", () => { - const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":[]}]}', staged); + const r = parseAndValidateSplitPlan('{"should_split":true,"commits":[{"message":"feat: a","files":[]}]}', staged); expect(r instanceof Failure).toBe(true); }); it("rejects duplicate paths", () => { - const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":["a.ts"]},{"message":"feat: again","files":["a.ts"]}]}', staged); + const r = parseAndValidateSplitPlan( + '{"should_split":true,"commits":[{"message":"feat: a","files":["a.ts"]},{"message":"feat: again","files":["a.ts"]}]}', + staged + ); expect(r instanceof Failure).toBe(true); }); it("rejects unknown path", () => { - const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: x","files":["missing.ts"]}]}', staged); + const r = parseAndValidateSplitPlan('{"should_split":true,"commits":[{"message":"feat: x","files":["missing.ts"]}]}', staged); expect(r instanceof Failure).toBe(true); }); it("appends leftover staged paths", () => { - const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":["a.ts"]}]}', staged); + const r = parseAndValidateSplitPlan('{"should_split":true,"commits":[{"message":"feat: a","files":["a.ts"]}]}', staged); expect(r instanceof Success).toBe(true); if (r instanceof Success) { expect(r.value.commits).toEqual([ @@ -68,4 +82,18 @@ describe("parseAndValidateSplitPlan", () => { ]); } }); + + it("folds leftover into first commit when should_split is false", () => { + const r = parseAndValidateSplitPlan('{"should_split":false,"commits":[{"message":"feat: a","files":["a.ts"]}]}', staged); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value.shouldSplit).toBe(false); + expect(r.value.commits).toEqual([{ message: "feat: a", files: ["a.ts", "b.ts", "c.ts"] }]); + } + }); + + it("rejects missing should_split", () => { + const r = parseAndValidateSplitPlan('{"commits":[{"message":"feat: a","files":["a.ts"]}]}', staged); + expect(r instanceof Failure).toBe(true); + }); }); From 99c4e1b89eb95d94f4fd1b230fac23bda4a9ddc3 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 05/41] Ask split prompts for JSON with should_split - Prefer splitting unrelated layers and emit JSON only. --- src/domain/commit/prompts.ts | 32 ++++++++++++++++++++++++++---- test/domain/commit/prompts.test.ts | 14 +++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/domain/commit/prompts.ts b/src/domain/commit/prompts.ts index a5aa286..e5e6094 100644 --- a/src/domain/commit/prompts.ts +++ b/src/domain/commit/prompts.ts @@ -276,8 +276,13 @@ function promptCustom(gitDiff: string, template: Maybe): string { } function getSplitPrompt(diff: string, files: readonly string[], convention: CommitConvention, customTemplate: Maybe = Nothing()): string { + const conventionPrompt = getPrompt(diff, convention, customTemplate).replace(/[\s\S]*?<\/output_instructions>/, ""); return ` - ${getPrompt(diff, convention, customTemplate)} + + Partition staged files into reviewable commits. Do not write one message for the whole diff. + A feature that touches unrelated layers is several commits. + + ${conventionPrompt} ${files.join("\n")} @@ -285,14 +290,33 @@ function getSplitPrompt(diff: string, files: readonly string[], convention: Comm Return ONE JSON object. First character "{", last "}". - {"commits":[{"message":"","files":["",...]}]} + {"should_split":,"commits":[{"message":"","files":["",...]}]} - Use only paths from . Exact strings. - Every staged path in exactly one commit. - - Prefer 2+ commits when files have distinct concerns; 1 commit is allowed if they are one change. - - Each message follows the active convention (SMALL/MEDIUM/LARGE shape). + - Prefer should_split=true when files (or groups) do not share one concern. Each group is its own commit. + - should_split=false only when every file is the same concern (implementation + its test, rename + callers). Then exactly 1 commit covering every staged path. + - Same feature across unrelated layers (CLI, domain, docs, unrelated tests) is still should_split=true. + - When unsure across 2+ areas, should_split=true with 2+ commits. + - Each message follows the active convention (SMALL/MEDIUM/LARGE shape) for that commit only. + + + src/cli/setup.ts +test/cli/setup.test.ts + {"should_split":false,"commits":[{"message":"Add split option to setup","files":["src/cli/setup.ts","test/cli/setup.test.ts"]}]} + + + src/cli/setup.ts +src/domain/split/plan.ts +test/domain/split/plan.test.ts + {"should_split":true,"commits":[{"message":"Add split option to setup","files":["src/cli/setup.ts"]},{"message":"Parse should_split in split plans","files":["src/domain/split/plan.ts","test/domain/split/plan.test.ts"]}]} + + + + Emit ONLY the JSON object. No prose, no markdown fences, no commentary. + `; } diff --git a/test/domain/commit/prompts.test.ts b/test/domain/commit/prompts.test.ts index 7e1f9e1..75e8ee2 100644 --- a/test/domain/commit/prompts.test.ts +++ b/test/domain/commit/prompts.test.ts @@ -37,6 +37,20 @@ describe("getSplitPrompt", () => { expect(prompt).toContain("foo.ts"); expect(prompt).toContain("bar.ts"); expect(prompt).toContain("commits"); + expect(prompt).toContain("should_split"); + }); + + it("forbids commit-message-only output and requires JSON-only output", () => { + const prompt = getSplitPrompt(DIFF, ["foo.ts", "bar.ts"], "conventional"); + expect(prompt).not.toMatch(/output ONLY the final commit message/i); + expect(prompt).toContain("Emit ONLY the JSON object"); + }); + + it("prefers split across unrelated layers", () => { + const prompt = getSplitPrompt(DIFF, ["foo.ts", "bar.ts"], "conventional"); + expect(prompt).toContain("Prefer should_split=true"); + expect(prompt).toContain("unrelated layers"); + expect(prompt).not.toContain("should_split=true only when"); }); }); From 60a4c251846f5c675096ebf06ef47c691eb24ba5 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 06/41] Generate split plans at minimum model effort - Add `withMinEffort` and apply it in `generateSplitPlan`. --- src/domain/llm/effort.ts | 22 +++++++++++++++++++--- src/domain/llm/router.ts | 3 ++- test/domain/llm/effort.test.ts | 27 +++++++++++++++++++++++++++ test/domain/llm/router.test.ts | 17 +++++++++++++++++ 4 files changed, 65 insertions(+), 4 deletions(-) create mode 100644 test/domain/llm/effort.test.ts diff --git a/src/domain/llm/effort.ts b/src/domain/llm/effort.ts index cf82871..0709764 100644 --- a/src/domain/llm/effort.ts +++ b/src/domain/llm/effort.ts @@ -1,4 +1,4 @@ -export { seedProviderConfig, withModel, selectEffortForProvider }; +export { seedProviderConfig, withModel, withMinEffort, selectEffortForProvider }; import { type Future } from "@/libs/future"; import { @@ -7,9 +7,10 @@ import { type OpenAIModelEffort, type XaiEffort, type AnthropicEffort, - type GeminiEffort + type GeminiEffort, + GEMINI_EFFORTS } from "@/domain/config/config"; -import { Nothing, type Maybe } from "@/libs/maybe"; +import { Just, Nothing, type Maybe } from "@/libs/maybe"; import { selectOpenAIEffort, selectAnthropicEffort, selectGeminiEffort, selectXaiEffort } from "@/infra/ui/effort-picker"; import { absurd } from "@/libs/types"; @@ -43,6 +44,21 @@ const withModel = (ai: ProviderConfig, model: string): ProviderConfig => { } }; +const withMinEffort = (config: ProviderConfig): ProviderConfig => { + switch (config.provider) { + case "openai": + return { provider: "openai", model: config.model, auth_method: config.auth_method, effort: Just("low") }; + case "anthropic": + return { provider: "anthropic", model: config.model, auth_method: config.auth_method, effort: Just("low") }; + case "gemini": + return { provider: "gemini", model: config.model, auth_method: config.auth_method, effort: Just(GEMINI_EFFORTS[0]) }; + case "xai": + return { provider: "xai", model: config.model, auth_method: config.auth_method, effort: Just("low") }; + default: + return absurd(config, "ProviderConfig"); + } +}; + const selectEffortForProvider = (current: ProviderConfig, modelEffort: Maybe = Nothing()): Future => { switch (current.provider) { case "openai": diff --git a/src/domain/llm/router.ts b/src/domain/llm/router.ts index 364f147..f944b33 100644 --- a/src/domain/llm/router.ts +++ b/src/domain/llm/router.ts @@ -25,6 +25,7 @@ import { getPrompt, getRefinePrompt, getBranchNamePrompt, getSplitPrompt } from import { parseAndValidateBranchSuggestions, type BranchSuggestion } from "@/domain/branch/suggestions"; import { parseAndValidateSplitPlan, type SplitPlan } from "@/domain/split/plan"; import { withTransientRetry } from "@/domain/llm/retry"; +import { withMinEffort } from "@/domain/llm/effort"; import { Maybe, Nothing } from "@/libs/maybe"; type GenerateContentParams = { @@ -146,7 +147,7 @@ const generateSplitPlan = ( customTemplate: Maybe ): Future => withTransientRetry(() => - generateContent(config, { prompt: getSplitPrompt(diff, files, convention, customTemplate) }).chain((gc) => + generateContent(withMinEffort(config), { prompt: getSplitPrompt(diff, files, convention, customTemplate) }).chain((gc) => resultToFuture(parseAndValidateSplitPlan(gc.text, files)).map((plan) => ({ plan, metadata: gc.metadata })) ) ); diff --git a/test/domain/llm/effort.test.ts b/test/domain/llm/effort.test.ts new file mode 100644 index 0000000..25e35cf --- /dev/null +++ b/test/domain/llm/effort.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { withMinEffort } from "@/domain/llm/effort"; +import { GEMINI_EFFORTS, type ProviderConfig } from "@/domain/config/config"; +import { Just, Nothing } from "@/libs/maybe"; + +const base = (provider: ProviderConfig["provider"]): ProviderConfig => + ({ + provider, + model: "m", + effort: Nothing(), + auth_method: { type: "api_key", content: "sk" } + }) as ProviderConfig; + +describe("withMinEffort", () => { + it("forces openai low", () => { + expect(withMinEffort(base("openai")).effort).toEqual(Just("low")); + }); + it("forces anthropic low", () => { + expect(withMinEffort(base("anthropic")).effort).toEqual(Just("low")); + }); + it("forces gemini MINIMAL", () => { + expect(withMinEffort(base("gemini")).effort).toEqual(Just(GEMINI_EFFORTS[0])); + }); + it("forces xai low", () => { + expect(withMinEffort(base("xai")).effort).toEqual(Just("low")); + }); +}); diff --git a/test/domain/llm/router.test.ts b/test/domain/llm/router.test.ts index 8e472a8..2207e5c 100644 --- a/test/domain/llm/router.test.ts +++ b/test/domain/llm/router.test.ts @@ -59,6 +59,7 @@ describe("generateSplitPlan", () => { it("parses provider JSON into commits", async () => { const { generateContentWithOpenAI } = await import("@/infra/llm/openai"); const json = JSON.stringify({ + should_split: true, commits: [ { message: "feat: a", files: ["a.ts"] }, { message: "feat: b", files: ["b.ts"] } @@ -67,10 +68,26 @@ describe("generateSplitPlan", () => { vi.mocked(generateContentWithOpenAI).mockReturnValue(Future.resolve({ text: json, tokens: Nothing(), effectiveEffort: Nothing() })); const result = await runFuture(generateSplitPlan(mockProvider("openai"), "diff", ["a.ts", "b.ts"], "conventional", Nothing())); + expect(result.plan.shouldSplit).toBe(true); expect(result.plan.commits).toEqual([ { message: "feat: a", files: ["a.ts"] }, { message: "feat: b", files: ["b.ts"] } ]); expect(result.metadata.model.provider).toBe("openai"); }); + + it("calls the provider with minimum effort", async () => { + const { generateContentWithOpenAI } = await import("@/infra/llm/openai"); + vi.mocked(generateContentWithOpenAI).mockClear(); + const json = JSON.stringify({ + should_split: false, + commits: [{ message: "feat: a", files: ["a.ts"] }] + }); + vi.mocked(generateContentWithOpenAI).mockReturnValue( + Future.resolve({ text: json, tokens: Nothing(), effectiveEffort: Just("low") }) + ); + const config = { ...mockProvider("openai"), effort: Just("high") } as ProviderConfig; + await runFuture(generateSplitPlan(config, "diff", ["a.ts"], "conventional", Nothing())); + expect(generateContentWithOpenAI.mock.calls[0]?.[0].effort).toEqual(Just("low")); + }); }); From 9d216947085b52694f7a234933a45e2992fc8e90 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 07/41] Prompt for auto-split during setup - Ask whether to analyze staged files and save `split_commits`. --- src/cli/setup.ts | 17 +++++++++++++++-- test/cli/setup.test.ts | 10 +++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/cli/setup.ts b/src/cli/setup.ts index 4473421..74a433e 100644 --- a/src/cli/setup.ts +++ b/src/cli/setup.ts @@ -23,6 +23,7 @@ type SetupPreferences = { readonly customTemplate: string | undefined; readonly provider: ProviderConfig["provider"]; readonly authMethod: "google_oauth" | "openai_oauth" | "xai_oauth" | "api_key" | "anthropic_setup_token"; + readonly splitCommits: boolean; }; class Setup { @@ -67,6 +68,16 @@ class Setup { customTemplate = template; } + const splitCommits = await p.select({ + message: "Analyze staged files and split them when they look independent?", + options: [ + { value: true, label: "Auto — split when needed (recommended)" }, + { value: false, label: "No — always one commit" } + ], + initialValue: true as const + }); + if (p.isCancel(splitCommits)) throw new Error("Setup cancelled"); + const authMethod = await p.select({ message: "Select authentication method:", options: getAuthMethodOptions(provider), @@ -79,7 +90,8 @@ class Setup { convention: convention, customTemplate, provider: provider, - authMethod: authMethod + authMethod: authMethod, + splitCommits }); }); } @@ -103,7 +115,8 @@ class Setup { return { ai, commit_convention: this.preferences.convention, - custom_template: this.preferences.customTemplate ? Just(this.preferences.customTemplate) : Nothing() + custom_template: this.preferences.customTemplate ? Just(this.preferences.customTemplate) : Nothing(), + split_commits: this.preferences.splitCommits }; } diff --git a/test/cli/setup.test.ts b/test/cli/setup.test.ts index ac746c3..b9567b0 100644 --- a/test/cli/setup.test.ts +++ b/test/cli/setup.test.ts @@ -62,18 +62,18 @@ vi.mock("@/infra/auth/anthropic", () => ({ validateAnthropicSetupToken: vi.fn() })); -/** The wizard asks provider, then convention, then auth method — in that order. */ -const scriptWizard = async (provider: string, convention: string, authMethod: string) => { +/** The wizard asks provider, then convention, then split, then auth method — in that order. */ +const scriptWizard = async (provider: string, convention: string, split: boolean, authMethod: string) => { const p = await import("@clack/prompts"); vi.mocked(p.select).mockReset(); - vi.mocked(p.select).mockResolvedValueOnce(provider).mockResolvedValueOnce(convention).mockResolvedValueOnce(authMethod); + vi.mocked(p.select).mockResolvedValueOnce(provider).mockResolvedValueOnce(convention).mockResolvedValueOnce(split).mockResolvedValueOnce(authMethod); }; describe("Setup.run", () => { beforeEach(() => vi.clearAllMocks()); it("saves config after wizard", async () => { - await scriptWizard("openai", "conventional", "api_key"); + await scriptWizard("openai", "conventional", false, "api_key"); const { saveConfig } = await import("@/infra/storage/config"); await runFuture(Setup.create().chain((s) => s.run())); @@ -82,7 +82,7 @@ describe("Setup.run", () => { }); it("saves an xai api_key config", async () => { - await scriptWizard("xai", "conventional", "api_key"); + await scriptWizard("xai", "conventional", false, "api_key"); const { saveConfig } = await import("@/infra/storage/config"); await runFuture(Setup.create().chain((s) => s.run())); From 4c8390b04393bec2a46c251194482b24d7378c27 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 08/41] Route commit through auto-split analysis - Analyze multi-file staged sets when `split_commits` is on. - Offer Split from the single-message prompt and reuse `Split.runPlan`. - Treat `commit split` as a forced multi-commit plan. --- src/cli/commit.ts | 75 +++++++++++++++++++++------- src/cli/parser.ts | 2 +- src/cli/split.ts | 13 ++++- test/cli/commit.test.ts | 108 +++++++++++++++++++++++++++++++++++++++- test/cli/split.test.ts | 2 + 5 files changed, 179 insertions(+), 21 deletions(-) diff --git a/src/cli/commit.ts b/src/cli/commit.ts index 54dcd9d..9e7b3eb 100644 --- a/src/cli/commit.ts +++ b/src/cli/commit.ts @@ -7,16 +7,24 @@ import * as repo from "@/infra/git/repo"; import { Future } from "@/libs/future"; import { loadConfig } from "@/infra/storage/config"; import { Setup } from "@/cli/setup"; +import { Split } from "@/cli/split"; import { type CommitConvention, type Config, type ProviderConfig } from "@/domain/config/config"; import { resolveProvider } from "@/domain/llm/auth-resolver"; -import { generateCommitMessage, refineCommitMessage, type GeneratedContent, type LlmRequestMetadata } from "@/domain/llm/router"; +import { + generateCommitMessage, + generateSplitPlan, + refineCommitMessage, + type GeneratedContent, + type LlmRequestMetadata, + type SplitPlanContent +} from "@/domain/llm/router"; import { Nothing, type Maybe, Just } from "@/libs/maybe"; import { loading } from "@/infra/ui/spinner"; import { renderCommitNote, renderPushNote } from "@/infra/ui/push-note"; import color from "picocolors"; -const USER_ACTIONS = ["commit_push", "commit", "regenerate", "adjust", "cancel"] as const; +const USER_ACTIONS = ["commit_push", "commit", "split", "regenerate", "adjust", "cancel"] as const; type UserAction = (typeof USER_ACTIONS)[number]; class Commit { @@ -39,14 +47,44 @@ class Commit { run(): Future { return repo .checkIsGitRepo() - .chain(() => this.diff()) - .chain((diff) => this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((message) => this.interact(diff, message))) + .chain(() => Future.concurrently({ diff: repo.getStagedDiff(), files: repo.listStagedPaths() })) + .chain(({ diff, files }) => this.route(diff, files)) .mapRej((e) => { p.log.error(color.red(e.message)); return e; }); } + private route(diff: string, files: readonly string[]): Future { + return this.config.split_commits && files.length >= 2 ? + loading( + "Analyzing staged changes...", + "Ready!", + generateSplitPlan(this.providerConfig, diff, files, this.config.commit_convention, this.config.custom_template) + ).chain((content) => this.followAnalysis(diff, files, content)) + : this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((message) => this.interact(diff, files, message)); + } + + private followAnalysis(diff: string, files: readonly string[], content: SplitPlanContent): Future { + const { plan, metadata } = content; + if (plan.shouldSplit && plan.commits.length >= 2) { + return Split.fromResolved(this.config, this.providerConfig).runPlan(diff, files, plan, metadata); + } + const first = plan.commits[0]; + if (first === undefined) { + return Future.reject(new Error("Split plan: expected at least 1 commit")); + } + return this.interact(diff, files, { text: first.message, metadata }); + } + + private startSplit(diff: string, files: readonly string[]): Future { + return loading( + "Generating split plan...", + "Split plan generated!", + generateSplitPlan(this.providerConfig, diff, files, this.config.commit_convention, this.config.custom_template) + ).chain((content) => Split.fromResolved(this.config, this.providerConfig).runPlan(diff, files, content.plan, content.metadata)); + } + diff(): Future { return repo.getStagedDiff(); } @@ -94,17 +132,19 @@ class Commit { ); } - interact(diff: string, generated: GeneratedContent): Future { - return this.promptAction(generated.text).chain((action) => { + interact(diff: string, files: readonly string[], generated: GeneratedContent): Future { + return this.promptAction(generated.text, files).chain((action) => { switch (action) { case "commit": return this.handleCommit(generated); case "commit_push": return this.handleCommitAndPush(generated); + case "split": + return this.startSplit(diff, files); case "regenerate": - return this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((msg) => this.interact(diff, msg)); + return this.route(diff, files); case "adjust": - return this.handleAdjust(diff, generated); + return this.handleAdjust(diff, files, generated); case "cancel": return Future.resolve(undefined); } @@ -116,18 +156,19 @@ class Commit { return msg.includes("non-fast-forward") || msg.includes("updates were rejected"); } - private promptAction(message: string): Future { + private promptAction(message: string, files: readonly string[]): Future { return Future.attemptP(async () => { p.note(message, "Proposed Commit Message"); const action = await p.select({ message: "What would you like to do?", options: [ - { value: "commit_push", label: "Commit & Push" }, - { value: "commit", label: "Commit" }, - { value: "regenerate", label: "Regenerate" }, - { value: "adjust", label: "Adjust" }, - { value: "cancel", label: "Cancel" } + { value: "commit_push" as const, label: "Commit & Push" }, + { value: "commit" as const, label: "Commit" }, + ...(files.length >= 2 ? [{ value: "split" as const, label: "Split" }] : []), + { value: "regenerate" as const, label: "Regenerate" }, + { value: "adjust" as const, label: "Adjust" }, + { value: "cancel" as const, label: "Cancel" } ] }); @@ -191,11 +232,11 @@ class Commit { }).chain((shouldForce) => (shouldForce ? this.push(request, undefined, false, true) : Future.resolve(undefined))); } - private handleAdjust(diff: string, generated: GeneratedContent): Future { + private handleAdjust(diff: string, files: readonly string[], generated: GeneratedContent): Future { return this.promptAdjustment().chain((maybeAdj) => maybeAdj instanceof Nothing ? - this.interact(diff, generated) - : this.refine(generated.text, maybeAdj.value, diff).chain((refined) => this.interact(diff, refined)) + this.interact(diff, files, generated) + : this.refine(generated.text, maybeAdj.value, diff).chain((refined) => this.interact(diff, files, refined)) ); } diff --git a/src/cli/parser.ts b/src/cli/parser.ts index c0dafb9..3161201 100644 --- a/src/cli/parser.ts +++ b/src/cli/parser.ts @@ -92,7 +92,7 @@ Usage: commit-tools [command] Commands: generate (default) Generate a commit message - split Split staged changes into multiple commits + split Force a multi-commit plan from staged changes branch Suggest branch names from local changes and create one new-branch Alias for branch setup Configure authentication and conventions diff --git a/src/cli/split.ts b/src/cli/split.ts index 6356817..8018f89 100644 --- a/src/cli/split.ts +++ b/src/cli/split.ts @@ -31,6 +31,7 @@ const commitOptions = (plan: SplitPlan): { value: number; label: string }[] => plan.commits.map((commit, index) => ({ value: index, label: `${index + 1}. ${commit.message}` })); const withEditedMessage = (plan: SplitPlan, index: number, message: string): SplitPlan => ({ + ...plan, commits: plan.commits.map((commit, i) => (i === index ? { message, files: commit.files } : commit)) }); @@ -48,7 +49,7 @@ const withMovedFile = (plan: SplitPlan, file: string, destIndex: number): SplitP } return commit; }); - return { commits: moved.filter((commit) => commit.files.length > 0) }; + return { commits: moved.filter((commit) => commit.files.length > 0), shouldSplit: plan.shouldSplit }; }; const withReorderedCommit = (plan: SplitPlan, fromIndex: number, toIndex: number): SplitPlan => { @@ -61,7 +62,7 @@ const withReorderedCommit = (plan: SplitPlan, fromIndex: number, toIndex: number return plan; } commits.splice(toIndex, 0, item); - return { commits }; + return { commits, shouldSplit: plan.shouldSplit }; }; class Split { @@ -81,6 +82,14 @@ class Split { .chain((config) => resolveProvider(config).map((ai) => new Split(config, ai))); } + static fromResolved(config: Config, providerConfig: ProviderConfig): Split { + return new Split(config, providerConfig); + } + + runPlan(diff: string, files: readonly string[], plan: SplitPlan, meta: LlmRequestMetadata): Future { + return this.interact(diff, files, plan, meta); + } + run(): Future { return repo .checkIsGitRepo() diff --git a/test/cli/commit.test.ts b/test/cli/commit.test.ts index 712146e..b55cada 100644 --- a/test/cli/commit.test.ts +++ b/test/cli/commit.test.ts @@ -22,6 +22,7 @@ vi.mock("@/domain/llm/auth-resolver", () => ({ vi.mock("@/infra/git/repo", () => ({ checkIsGitRepo: vi.fn(() => Future.resolve(undefined)), getStagedDiff: vi.fn(() => Future.resolve("staged diff")), + listStagedPaths: vi.fn(() => Future.resolve(["a.ts"])), performCommit: vi.fn(() => Future.resolve("\n 1 file changed\n")), findCommitMetadata: vi.fn() })); @@ -32,6 +33,15 @@ vi.mock("@/domain/llm/router", () => ({ metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } }) ), + generateSplitPlan: vi.fn(() => + Future.resolve({ + plan: { + shouldSplit: false, + commits: [{ message: "feat: one", files: ["a.ts", "b.ts"] }] + }, + metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } + }) + ), refineCommitMessage: vi.fn() })); vi.mock("@clack/prompts", () => ({ @@ -51,9 +61,10 @@ vi.mock("@/infra/ui/spinner", () => ({ loading: vi.fn((_a: string, _b: string, f: Future) => f as Future) })); -const config = (): ConfigValue => ({ +const config = (split_commits = false): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits, ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } }); @@ -63,9 +74,22 @@ describe("Commit.run", () => { const storage = await import("@/infra/storage/config"); vi.mocked(storage.loadConfig).mockReturnValue(Future.resolve(config())); const repo = await import("@/infra/git/repo"); + vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts"])); vi.mocked(repo.findCommitMetadata).mockReturnValue( Future.resolve(Just({ hash: "h", short: "h", subject: "feat: generated", authorName: "t", authorEmail: "t@t.com", date: new Date() })) ); + const router = await import("@/domain/llm/router"); + vi.mocked(router.generateSplitPlan).mockReturnValue( + Future.resolve({ + plan: { + shouldSplit: false, + commits: [{ message: "feat: one", files: ["a.ts", "b.ts"] }] + }, + metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } + }) + ); + const prompts = await import("@clack/prompts"); + vi.mocked(prompts.select).mockResolvedValue("commit"); }); it("commits when user selects commit", async () => { @@ -73,4 +97,86 @@ describe("Commit.run", () => { const repo = await import("@/infra/git/repo"); expect(repo.performCommit).toHaveBeenCalledWith("feat: generated"); }); + + it("does not analyze when split is disabled and two files are staged", async () => { + const repo = await import("@/infra/git/repo"); + vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); + + await runFuture(Commit.create().chain((c) => c.run())); + + const router = await import("@/domain/llm/router"); + expect(router.generateCommitMessage).toHaveBeenCalled(); + expect(router.generateSplitPlan).not.toHaveBeenCalled(); + expect(repo.performCommit).toHaveBeenCalledWith("feat: generated"); + }); + + it("applies a split plan when auto-analysis says to split", async () => { + const storage = await import("@/infra/storage/config"); + vi.mocked(storage.loadConfig).mockReturnValue(Future.resolve(config(true))); + const repo = await import("@/infra/git/repo"); + vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); + const router = await import("@/domain/llm/router"); + vi.mocked(router.generateSplitPlan).mockReturnValue( + Future.resolve({ + plan: { + shouldSplit: true, + commits: [ + { message: "msg one", files: ["a.ts"] }, + { message: "msg two", files: ["b.ts"] } + ] + }, + metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } + }) + ); + const prompts = await import("@clack/prompts"); + vi.mocked(prompts.select).mockResolvedValue("apply"); + + await runFuture(Commit.create().chain((c) => c.run())); + + expect(router.generateSplitPlan).toHaveBeenCalled(); + expect(router.generateCommitMessage).not.toHaveBeenCalled(); + expect(repo.performCommit).toHaveBeenNthCalledWith(1, "msg one", ["a.ts"]); + expect(repo.performCommit).toHaveBeenNthCalledWith(2, "msg two", ["b.ts"]); + }); + + it("commits the single analysis message when shouldSplit is false", async () => { + const storage = await import("@/infra/storage/config"); + vi.mocked(storage.loadConfig).mockReturnValue(Future.resolve(config(true))); + const repo = await import("@/infra/git/repo"); + vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); + + await runFuture(Commit.create().chain((c) => c.run())); + + const router = await import("@/domain/llm/router"); + expect(router.generateSplitPlan).toHaveBeenCalled(); + expect(router.generateCommitMessage).not.toHaveBeenCalled(); + expect(repo.performCommit).toHaveBeenCalledWith("feat: one"); + }); + + it("forces a split plan when the user picks Split", async () => { + const repo = await import("@/infra/git/repo"); + vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); + const router = await import("@/domain/llm/router"); + vi.mocked(router.generateSplitPlan).mockReturnValue( + Future.resolve({ + plan: { + shouldSplit: true, + commits: [ + { message: "msg one", files: ["a.ts"] }, + { message: "msg two", files: ["b.ts"] } + ] + }, + metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } + }) + ); + const prompts = await import("@clack/prompts"); + vi.mocked(prompts.select).mockResolvedValueOnce("split").mockResolvedValueOnce("apply"); + + await runFuture(Commit.create().chain((c) => c.run())); + + expect(router.generateCommitMessage).toHaveBeenCalled(); + expect(router.generateSplitPlan).toHaveBeenCalled(); + expect(repo.performCommit).toHaveBeenNthCalledWith(1, "msg one", ["a.ts"]); + expect(repo.performCommit).toHaveBeenNthCalledWith(2, "msg two", ["b.ts"]); + }); }); diff --git a/test/cli/split.test.ts b/test/cli/split.test.ts index 54c13c9..9eff795 100644 --- a/test/cli/split.test.ts +++ b/test/cli/split.test.ts @@ -30,6 +30,7 @@ vi.mock("@/domain/llm/router", () => ({ generateSplitPlan: vi.fn(() => Future.resolve({ plan: { + shouldSplit: true, commits: [ { message: "msg one", files: ["a.ts"] }, { message: "msg two", files: ["b.ts"] } @@ -59,6 +60,7 @@ vi.mock("@/infra/ui/spinner", () => ({ const config = (): ConfigValue => ({ commit_convention: "conventional", custom_template: Nothing(), + split_commits: false, ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } }); From f511204a101eac1d570551a8ed289947ad1ee806 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:06:36 -0300 Subject: [PATCH 09/41] Document auto-split setup and commit split behavior --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b093260..a8b78b3 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ You will be prompted to choose: - Anthropic: Claude setup-token or API key - xAI: Sign in with Grok or API key - **Commit convention**: Conventional, Imperative, or Custom +- **Split commits**: whether `commit` should analyze staged files and open a multi-commit plan when they look independent If you want to use your claude.ai subscription with Anthropic, run `claude setup-token` in another terminal first, then paste the generated setup-token during `commit setup`. @@ -160,7 +161,7 @@ Or explicitly: commit generate ``` -`commit split` for a multi-commit plan. +With split enabled in setup, `commit` analyzes staged files and opens a multi-commit plan when they look independent. `commit split` always opens that plan. From a single-message prompt you can also pick Split. ### System Checks @@ -200,7 +201,7 @@ commit --help | ------------------------ | ------------------------------------------------- | | `commit` | Generate a commit message (default) | | `commit generate` | Generate a commit message | -| `commit split` | Split staged changes into multiple commits | +| `commit split` | Force a multi-commit plan from staged changes | | `commit setup` | Configure authentication and conventions | | `commit login` | Alias for setup — re-authenticate | | `commit doctor` | Check installation and environment | From 38d3e0b3941d50454fc5f36c92fd26dd6a7dd2f8 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 19:07:13 -0300 Subject: [PATCH 10/41] Collapse generateContentWithOpenAI mock onto one line --- test/domain/llm/router.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/domain/llm/router.test.ts b/test/domain/llm/router.test.ts index 2207e5c..7aebdb5 100644 --- a/test/domain/llm/router.test.ts +++ b/test/domain/llm/router.test.ts @@ -83,9 +83,7 @@ describe("generateSplitPlan", () => { should_split: false, commits: [{ message: "feat: a", files: ["a.ts"] }] }); - vi.mocked(generateContentWithOpenAI).mockReturnValue( - Future.resolve({ text: json, tokens: Nothing(), effectiveEffort: Just("low") }) - ); + vi.mocked(generateContentWithOpenAI).mockReturnValue(Future.resolve({ text: json, tokens: Nothing(), effectiveEffort: Just("low") })); const config = { ...mockProvider("openai"), effort: Just("high") } as ProviderConfig; await runFuture(generateSplitPlan(config, "diff", ["a.ts"], "conventional", Nothing())); expect(generateContentWithOpenAI.mock.calls[0]?.[0].effort).toEqual(Just("low")); From 258b03ebde374fdbb04f67d23afe7de92a050ad7 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:02:17 -0300 Subject: [PATCH 11/41] Commit split groups from the index not the worktree - Isolate path-limited commits on a temporary index so `git commit` records staged blobs. - Leave unstaged hunks in the worktree and other staged paths in the real index. - Cover staged-plus-unstaged on the same path in the repo integration tests. --- src/infra/git/repo.ts | 44 ++++++++++++++++++++++--- src/infra/shell.ts | 7 ++-- test/infra/git/repo.integration.test.ts | 17 ++++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index c205971..d8fc34d 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -29,9 +29,9 @@ import { Just, Nothing, type Maybe } from "@/libs/maybe"; import { type Result, Failure } from "@/libs/result"; import { absurd } from "@/libs/types"; import { type BaseLookupError } from "@/infra/git/parsers"; -import { execBin } from "@/infra/shell"; +import { execBin, type ExecResult } from "@/infra/shell"; import * as Decoder from "@/libs/json/decoder"; -import { unlink, writeFile } from "node:fs/promises"; +import { copyFile, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { @@ -59,8 +59,8 @@ type PushResult = { range: Maybe; }; -const execGitChecked = (args: string[], fallbackMsg: string): Future => - execBin("git", args).chain((result) => +const execGitChecked = (args: string[], fallbackMsg: string, env?: NodeJS.ProcessEnv): Future => + execBin("git", args, env).chain((result) => result.either( (failure) => Future.reject(new Error(commandFailureMessage(failure, fallbackMsg))), ({ stdout }) => Future.resolve(stdout) @@ -106,6 +106,40 @@ const createAndSwitchBranch = (name: string): Future => const getWorkTreeRoot = (): Future => execGitChecked(["rev-parse", "--show-toplevel"], "Failed to resolve git directory").map((s) => s.trim()); +const indexEnv = (indexFile: string): NodeJS.ProcessEnv => ({ GIT_INDEX_FILE: indexFile }); + +const splitNulPaths = (stdout: string): readonly string[] => stdout.split("\0").filter((p) => p.length > 0); + +const copyIndexFile = (root: string, dest: string): Future => + execGitChecked(["-C", root, "rev-parse", "--absolute-git-dir"], "Failed to resolve git directory").chain((gitDir) => + Future.attemptP(() => copyFile(join(gitDir.trim(), "index"), dest)) + ); + +const listStagedPathsNoRenames = (root: string): Future => + execGitChecked(["-C", root, "diff", "--staged", "--name-only", "--no-renames", "-z"], "Failed to list staged files").map(splitNulPaths); + +const resetIndexPaths = (root: string, indexFile: string, paths: readonly string[]): Future => + paths.length === 0 ? + Future.resolve(undefined) + : execGitChecked(["-C", root, "reset", "-q", "HEAD", "--", ...paths], "Failed to isolate staged paths", indexEnv(indexFile)).map(() => {}); + +const commitIsolatedPaths = (root: string, messageFile: string, paths: readonly string[]): Future => { + const tmpIndex = join(tmpdir(), `commit-index-${Date.now()}`); + return Future.bracket( + copyIndexFile(root, tmpIndex), + () => Future.attemptP(() => unlink(tmpIndex).catch(() => {})), + () => + listStagedPathsNoRenames(root).chain((staged) => { + const keep = new Set(paths); + return resetIndexPaths( + root, + tmpIndex, + staged.filter((path) => !keep.has(path)) + ).chain(() => execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex))); + }) + ); +}; + const performCommit = (message: string, paths: readonly string[] = []): Future => { const tmpPath = join(tmpdir(), `commit-msg-${Date.now()}.txt`); return getWorkTreeRoot() @@ -113,7 +147,7 @@ const performCommit = (message: string, paths: readonly string[] = []): Future writeFile(tmpPath, message, "utf-8")), () => Future.attemptP(() => unlink(tmpPath).catch(() => {})), - () => execBin("git", paths.length > 0 ? ["-C", root, "commit", "-F", tmpPath, "--", ...paths] : ["-C", root, "commit", "-F", tmpPath]) + () => (paths.length > 0 ? commitIsolatedPaths(root, tmpPath, paths) : execBin("git", ["-C", root, "commit", "-F", tmpPath])) ) ) .chain((result) => diff --git a/src/infra/shell.ts b/src/infra/shell.ts index c6c937d..05a0b7c 100644 --- a/src/infra/shell.ts +++ b/src/infra/shell.ts @@ -16,9 +16,12 @@ const commandResult = (output: CommandOutput, exitCode: number | null, signal: N Success(output) : Failure({ output, error: exitCodeError(exitCode, signal) }); -const execBin = (bin: string, args: string[]): Future => +const execBin = (bin: string, args: string[], env?: NodeJS.ProcessEnv): Future => Future.create((reject, resolve) => { - const proc = spawn(bin, args, { stdio: ["pipe", "pipe", "pipe"] }); + const proc = spawn(bin, args, { + stdio: ["pipe", "pipe", "pipe"], + env: env === undefined ? undefined : { ...process.env, ...env } + }); let stdout = ""; let stderr = ""; diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 7008848..4037b82 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -126,6 +126,23 @@ describe("git repo integration", () => { } }); + it("performCommit with paths records the staged blob not the worktree", async () => { + const { dir, run } = createTempGitRepo({ staged: true }); + writeFileSync(join(dir, "file.txt"), "hello unstaged secret\n"); + writeFileSync(join(dir, "other.txt"), "other\n"); + run("add other.txt"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: staged only", ["file.txt"])); + expect(run("show HEAD:file.txt")).toBe("hello world\n"); + expect(run("diff --staged --name-only").trim()).toBe("other.txt"); + expect(run("diff -- file.txt")).toContain("hello unstaged secret"); + } finally { + chdir(prev); + } + }); + it("performCommit pathspecs resolve from worktree root when cwd is a subdirectory", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "app")); From 98648871d138baf123bba50fc8eb7024648d8195 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:02:53 -0300 Subject: [PATCH 12/41] List both sides of staged renames - Pass `--no-renames` to `listStagedPaths` so a `git mv` includes the deleted source. - Assert apply of a staged rename leaves no leftover deletion in the index. --- src/infra/git/repo.ts | 8 ++++---- test/infra/git/repo.integration.test.ts | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index d8fc34d..d7bba13 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -67,6 +67,8 @@ const execGitChecked = (args: string[], fallbackMsg: string, env?: NodeJS.Proces ) ); +const splitNulPaths = (stdout: string): readonly string[] => stdout.split("\0").filter((p) => p.length > 0); + const checkIsGitRepo = (): Future => execGitChecked(["rev-parse", "--is-inside-work-tree"], "Not a git repository").map(() => {}); const getStagedDiff = (): Future => @@ -75,8 +77,8 @@ const getStagedDiff = (): Future => ); const listStagedPaths = (): Future => - execGitChecked(["diff", "--staged", "--name-only", "-z"], "Failed to list staged files").chain((stdout) => { - const files = stdout.split("\0").filter((p) => p.length > 0); + execGitChecked(["diff", "--staged", "--name-only", "--no-renames", "-z"], "Failed to list staged files").chain((stdout) => { + const files = splitNulPaths(stdout); return files.length > 0 ? Future.resolve(files) : Future.reject(new Error("No staged changes found")); @@ -108,8 +110,6 @@ const getWorkTreeRoot = (): Future => const indexEnv = (indexFile: string): NodeJS.ProcessEnv => ({ GIT_INDEX_FILE: indexFile }); -const splitNulPaths = (stdout: string): readonly string[] => stdout.split("\0").filter((p) => p.length > 0); - const copyIndexFile = (root: string, dest: string): Future => execGitChecked(["-C", root, "rev-parse", "--absolute-git-dir"], "Failed to resolve git directory").chain((gitDir) => Future.attemptP(() => copyFile(join(gitDir.trim(), "index"), dest)) diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 4037b82..347784d 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -126,6 +126,22 @@ describe("git repo integration", () => { } }); + it("listStagedPaths includes both sides of a rename and apply leaves no staged deletion", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + run("mv file.txt renamed.txt"); + const prev = cwd(); + chdir(dir); + try { + const staged = await runFuture(repo.listStagedPaths()); + expect([...staged].sort()).toEqual(["file.txt", "renamed.txt"]); + await runFuture(repo.performCommit("feat: rename file", staged)); + expect(run("diff --staged --name-only").trim()).toBe(""); + expect(run("ls-files").trim()).toBe("renamed.txt"); + } finally { + chdir(prev); + } + }); + it("performCommit with paths records the staged blob not the worktree", async () => { const { dir, run } = createTempGitRepo({ staged: true }); writeFileSync(join(dir, "file.txt"), "hello unstaged secret\n"); From b3dd3af2c7624b52cb4cef8680f532d212dd3fa9 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:03:10 -0300 Subject: [PATCH 13/41] Unblock typecheck on the split-plan decoder - Type `nonEmptyCommits` so `D.object` infers `Decoder`. - Read OpenAI mock calls through `vi.mocked` in the router test. --- src/domain/split/plan.ts | 6 +++++- test/domain/llm/router.test.ts | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/domain/split/plan.ts b/src/domain/split/plan.ts index 8b6cb07..9016007 100644 --- a/src/domain/split/plan.ts +++ b/src/domain/split/plan.ts @@ -23,9 +23,13 @@ const splitCommitDecoder: D.Decoder = D.object({ files: nonEmptyFiles }); +const nonEmptyCommits: D.Decoder = D.array(splitCommitDecoder).chain((xs) => + xs.length === 0 ? D.fail("expected at least 1 commit") : D.succeed(xs) +); + const splitPlanDecoder: D.Decoder = D.object({ should_split: D.boolean, - commits: D.array(splitCommitDecoder).chain((xs) => (xs.length === 0 ? D.fail("expected at least 1 commit") : D.succeed(xs))) + commits: nonEmptyCommits }).map(({ should_split, commits }) => ({ shouldSplit: should_split, commits })); const stripOptionalJsonFence = (s: string): string => { diff --git a/test/domain/llm/router.test.ts b/test/domain/llm/router.test.ts index 7aebdb5..eb8a466 100644 --- a/test/domain/llm/router.test.ts +++ b/test/domain/llm/router.test.ts @@ -86,6 +86,6 @@ describe("generateSplitPlan", () => { vi.mocked(generateContentWithOpenAI).mockReturnValue(Future.resolve({ text: json, tokens: Nothing(), effectiveEffort: Just("low") })); const config = { ...mockProvider("openai"), effort: Just("high") } as ProviderConfig; await runFuture(generateSplitPlan(config, "diff", ["a.ts"], "conventional", Nothing())); - expect(generateContentWithOpenAI.mock.calls[0]?.[0].effort).toEqual(Just("low")); + expect(vi.mocked(generateContentWithOpenAI).mock.calls[0]?.[0].effort).toEqual(Just("low")); }); }); From f2dde893c24b79389209eff67d2a5738eb2ed87c Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:03:45 -0300 Subject: [PATCH 14/41] Collapse no-split plans to one commit - Fold leftover paths and extra groups into the first commit when `should_split` is false. - Cover a fully assigned multi-commit payload that still says not to split. --- src/domain/split/plan.ts | 24 ++++++++++++++---------- test/domain/split/plan.test.ts | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/domain/split/plan.ts b/src/domain/split/plan.ts index 9016007..d43d511 100644 --- a/src/domain/split/plan.ts +++ b/src/domain/split/plan.ts @@ -66,6 +66,17 @@ const parseSplitPlan = (raw: string): Result => { return D.decode(json, splitPlanDecoder).mapFailure((msg) => new Error(`Split plan: ${msg}`)); }; +const collapseToSingleCommit = (commits: readonly SplitCommit[], leftover: readonly string[]): Result => { + const [first, ...rest] = commits; + if (first === undefined) { + return Failure(new Error("Split plan: expected at least 1 commit")); + } + return Success({ + shouldSplit: false, + commits: [{ message: first.message, files: [...first.files, ...rest.flatMap((commit) => commit.files), ...leftover] }] + }); +}; + const validateSplitPlan = (plan: SplitPlan, stagedFiles: readonly string[]): Result => { const staged = new Set(stagedFiles); const seen = new Set(); @@ -81,19 +92,12 @@ const validateSplitPlan = (plan: SplitPlan, stagedFiles: readonly string[]): Res } } const leftover = stagedFiles.filter((file) => !seen.has(file)); + if (!plan.shouldSplit) { + return collapseToSingleCommit(plan.commits, leftover); + } if (leftover.length === 0) { return Success(plan); } - if (!plan.shouldSplit) { - const [first, ...rest] = plan.commits; - if (first === undefined) { - return Failure(new Error("Split plan: expected at least 1 commit")); - } - return Success({ - shouldSplit: false, - commits: [{ message: first.message, files: [...first.files, ...leftover] }, ...rest] - }); - } return Success({ shouldSplit: true, commits: [...plan.commits, { message: REMAINING_STAGED_MESSAGE, files: leftover }] diff --git a/test/domain/split/plan.test.ts b/test/domain/split/plan.test.ts index 1e8ecf9..52f228c 100644 --- a/test/domain/split/plan.test.ts +++ b/test/domain/split/plan.test.ts @@ -83,6 +83,22 @@ describe("parseAndValidateSplitPlan", () => { } }); + it("collapses extra commits when should_split is false", () => { + const raw = JSON.stringify({ + should_split: false, + commits: [ + { message: "feat: a", files: ["a.ts"] }, + { message: "feat: b", files: ["b.ts", "c.ts"] } + ] + }); + const r = parseAndValidateSplitPlan(raw, staged); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value.shouldSplit).toBe(false); + expect(r.value.commits).toEqual([{ message: "feat: a", files: ["a.ts", "b.ts", "c.ts"] }]); + } + }); + it("folds leftover into first commit when should_split is false", () => { const r = parseAndValidateSplitPlan('{"should_split":false,"commits":[{"message":"feat: a","files":["a.ts"]}]}', staged); expect(r instanceof Success).toBe(true); From 09a484b4664c5487dd875fab258db58149a9f1b2 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:04:33 -0300 Subject: [PATCH 15/41] Strip only the trailing output instructions from split prompts - Drop the last `` block so a matching tag in the staged diff stays in the prompt. - Assert a hunk that contains those tags is kept and the commit-message trailer is not. --- src/domain/commit/prompts.ts | 4 +++- test/domain/commit/prompts.test.ts | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/domain/commit/prompts.ts b/src/domain/commit/prompts.ts index e5e6094..7a09fea 100644 --- a/src/domain/commit/prompts.ts +++ b/src/domain/commit/prompts.ts @@ -276,7 +276,9 @@ function promptCustom(gitDiff: string, template: Maybe): string { } function getSplitPrompt(diff: string, files: readonly string[], convention: CommitConvention, customTemplate: Maybe = Nothing()): string { - const conventionPrompt = getPrompt(diff, convention, customTemplate).replace(/[\s\S]*?<\/output_instructions>/, ""); + const basePrompt = getPrompt(diff, convention, customTemplate); + const outputInstructionsStart = basePrompt.lastIndexOf(""); + const conventionPrompt = outputInstructionsStart >= 0 ? basePrompt.slice(0, outputInstructionsStart) : basePrompt; return ` Partition staged files into reviewable commits. Do not write one message for the whole diff. diff --git a/test/domain/commit/prompts.test.ts b/test/domain/commit/prompts.test.ts index 75e8ee2..3de1039 100644 --- a/test/domain/commit/prompts.test.ts +++ b/test/domain/commit/prompts.test.ts @@ -46,6 +46,14 @@ describe("getSplitPrompt", () => { expect(prompt).toContain("Emit ONLY the JSON object"); }); + it("keeps a diff that contains output_instructions tags", () => { + const diff = "diff --git a/x b/x\n+\n+keep this hunk\n+"; + const prompt = getSplitPrompt(diff, ["x"], "conventional"); + expect(prompt).toContain("keep this hunk"); + expect(prompt).not.toMatch(/output ONLY the final commit message/i); + expect(prompt).toContain("Emit ONLY the JSON object"); + }); + it("prefers split across unrelated layers", () => { const prompt = getSplitPrompt(DIFF, ["foo.ts", "bar.ts"], "conventional"); expect(prompt).toContain("Prefer should_split=true"); From 8258b71ce482a58a555ce2e38d7b68550c9bfb51 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:04:33 -0300 Subject: [PATCH 16/41] Parse split JSON from the should_split object - Slice from `{"should_split"` so a preview brace in surrounding prose cannot poison parse. - Cover a schema-preview prefix before the real plan object. --- src/domain/split/plan.ts | 3 ++- test/domain/split/plan.test.ts | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/domain/split/plan.ts b/src/domain/split/plan.ts index d43d511..02cb320 100644 --- a/src/domain/split/plan.ts +++ b/src/domain/split/plan.ts @@ -47,7 +47,8 @@ const stripOptionalJsonFence = (s: string): string => { }; const extractJsonObject = (s: string): string => { - const start = s.indexOf("{"); + const match = s.match(/\{\s*"should_split"\s*:/); + const start = match?.index ?? s.indexOf("{"); const end = s.lastIndexOf("}"); if (start === -1 || end <= start) { return s; diff --git a/test/domain/split/plan.test.ts b/test/domain/split/plan.test.ts index 52f228c..a05c08b 100644 --- a/test/domain/split/plan.test.ts +++ b/test/domain/split/plan.test.ts @@ -39,6 +39,11 @@ describe("parseAndValidateSplitPlan", () => { expect(r instanceof Success).toBe(true); }); + it("skips a preview brace before the plan object", () => { + const r = parseAndValidateSplitPlan('First character "{", last "}".\n' + threeCommits, staged); + expect(r instanceof Success).toBe(true); + }); + it("rejects invalid JSON", () => { const r = parseAndValidateSplitPlan("feat: add foo", staged); expect(r instanceof Failure).toBe(true); From 20593b264d28f749bf018caf58598cbb8df42e0a Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:04:33 -0300 Subject: [PATCH 17/41] Keep regenerate on the single-message flow - Call `generate` from Regenerate instead of `route` so split analysis cannot hijack the prompt. - Assert regenerate still commits the new single message when `split_commits` is on. --- src/cli/commit.ts | 2 +- test/cli/commit.test.ts | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/cli/commit.ts b/src/cli/commit.ts index 9e7b3eb..7b589c7 100644 --- a/src/cli/commit.ts +++ b/src/cli/commit.ts @@ -142,7 +142,7 @@ class Commit { case "split": return this.startSplit(diff, files); case "regenerate": - return this.route(diff, files); + return this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((msg) => this.interact(diff, files, msg)); case "adjust": return this.handleAdjust(diff, files, generated); case "cancel": diff --git a/test/cli/commit.test.ts b/test/cli/commit.test.ts index b55cada..7c2ca8d 100644 --- a/test/cli/commit.test.ts +++ b/test/cli/commit.test.ts @@ -153,6 +153,22 @@ describe("Commit.run", () => { expect(repo.performCommit).toHaveBeenCalledWith("feat: one"); }); + it("regenerates the single message instead of re-routing into split", async () => { + const storage = await import("@/infra/storage/config"); + vi.mocked(storage.loadConfig).mockReturnValue(Future.resolve(config(true))); + const repo = await import("@/infra/git/repo"); + vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); + const prompts = await import("@clack/prompts"); + vi.mocked(prompts.select).mockResolvedValueOnce("regenerate").mockResolvedValueOnce("commit"); + + await runFuture(Commit.create().chain((c) => c.run())); + + const router = await import("@/domain/llm/router"); + expect(router.generateSplitPlan).toHaveBeenCalledTimes(1); + expect(router.generateCommitMessage).toHaveBeenCalled(); + expect(repo.performCommit).toHaveBeenCalledWith("feat: generated"); + }); + it("forces a split plan when the user picks Split", async () => { const repo = await import("@/infra/git/repo"); vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); From 9e2bc1bbf2e29ce0a63ba5e36d081e684f3c9274 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:06:05 -0300 Subject: [PATCH 18/41] Lower split-plan validation below the complexity cap - Move path membership checks out of `validateSplitPlan` so lint:ci stays at the repo limit. --- src/domain/split/plan.ts | 48 +++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/domain/split/plan.ts b/src/domain/split/plan.ts index 02cb320..7cab464 100644 --- a/src/domain/split/plan.ts +++ b/src/domain/split/plan.ts @@ -78,32 +78,44 @@ const collapseToSingleCommit = (commits: readonly SplitCommit[], leftover: reado }); }; -const validateSplitPlan = (plan: SplitPlan, stagedFiles: readonly string[]): Result => { +const takePath = (file: string, staged: Set, seen: Set): Result => { + if (seen.has(file)) { + return Failure(new Error(`Split plan: duplicate path: ${file}`)); + } + if (!staged.has(file)) { + return Failure(new Error(`Split plan: unknown path: ${file}`)); + } + seen.add(file); + return Success(undefined); +}; + +const leftoverStagedPaths = (plan: SplitPlan, stagedFiles: readonly string[]): Result => { const staged = new Set(stagedFiles); const seen = new Set(); for (const commit of plan.commits) { for (const file of commit.files) { - if (seen.has(file)) { - return Failure(new Error(`Split plan: duplicate path: ${file}`)); + const taken = takePath(file, staged, seen); + if (taken instanceof Failure) { + return Failure(taken.error); } - if (!staged.has(file)) { - return Failure(new Error(`Split plan: unknown path: ${file}`)); - } - seen.add(file); } } - const leftover = stagedFiles.filter((file) => !seen.has(file)); - if (!plan.shouldSplit) { - return collapseToSingleCommit(plan.commits, leftover); - } - if (leftover.length === 0) { - return Success(plan); - } - return Success({ - shouldSplit: true, - commits: [...plan.commits, { message: REMAINING_STAGED_MESSAGE, files: leftover }] - }); + return Success(stagedFiles.filter((file) => !seen.has(file))); }; +const validateSplitPlan = (plan: SplitPlan, stagedFiles: readonly string[]): Result => + leftoverStagedPaths(plan, stagedFiles).chain((leftover) => { + if (!plan.shouldSplit) { + return collapseToSingleCommit(plan.commits, leftover); + } + if (leftover.length === 0) { + return Success(plan); + } + return Success({ + shouldSplit: true, + commits: [...plan.commits, { message: REMAINING_STAGED_MESSAGE, files: leftover }] + }); + }); + const parseAndValidateSplitPlan = (raw: string, stagedFiles: readonly string[]): Result => parseSplitPlan(raw).chain((plan) => validateSplitPlan(plan, stagedFiles)); From 24b5cc88d35287c1f8266b0db9e5f897ce088982 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:19:25 -0300 Subject: [PATCH 19/41] Isolate pathspec commits on unborn branches - Unstage leftover files with `git rm --cached` on the temp index when `HEAD` is missing. - Cover `performCommit` pathspecs after `git init` with two staged files. --- src/infra/git/repo.ts | 11 ++++++++++- test/infra/git/repo.integration.test.ts | 25 ++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index d7bba13..f4e3c22 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -121,7 +121,16 @@ const listStagedPathsNoRenames = (root: string): Future => paths.length === 0 ? Future.resolve(undefined) - : execGitChecked(["-C", root, "reset", "-q", "HEAD", "--", ...paths], "Failed to isolate staged paths", indexEnv(indexFile)).map(() => {}); + : execBin("git", ["-C", root, "rev-parse", "-q", "--verify", "HEAD"]).chain((head) => + execGitChecked( + head.either( + () => ["-C", root, "rm", "--cached", "-q", "--", ...paths], + () => ["-C", root, "reset", "-q", "HEAD", "--", ...paths] + ), + "Failed to isolate staged paths", + indexEnv(indexFile) + ).map(() => {}) + ); const commitIsolatedPaths = (root: string, messageFile: string, paths: readonly string[]): Future => { const tmpIndex = join(tmpdir(), `commit-index-${Date.now()}`); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 347784d..4b7ab39 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "vitest"; import { chdir, cwd } from "node:process"; -import { mkdirSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { execSync } from "node:child_process"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import { runFuture } from "@test/helpers/run-future"; import { createTempGitRepo } from "@test/helpers/temp-git-repo"; @@ -159,6 +161,27 @@ describe("git repo integration", () => { } }); + it("performCommit with pathspecs isolates files on an unborn branch", async () => { + const dir = mkdtempSync(join(tmpdir(), "commit-tools-git-")); + const run = (args: string) => execSync(`git ${args}`, { cwd: dir, encoding: "utf-8" }); + run("init -b main"); + run('config user.email "test@example.com"'); + run('config user.name "Test"'); + writeFileSync(join(dir, "a.txt"), "a\n"); + writeFileSync(join(dir, "b.txt"), "b\n"); + run("add a.txt b.txt"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: first", ["a.txt"])); + expect(run("diff --staged --name-only").trim()).toBe("b.txt"); + expect((await runFuture(repo.getCommitMetadata())).subject).toBe("feat: first"); + expect(run("show HEAD:a.txt")).toBe("a\n"); + } finally { + chdir(prev); + } + }); + it("performCommit pathspecs resolve from worktree root when cwd is a subdirectory", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "app")); From 8be61f80bdb74d6246ee297598c1944a161d6b33 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:31:43 -0300 Subject: [PATCH 20/41] Fix isolated commits for hooks and dirty unborn trees - Force `git rm --cached` with `-f` so leftover unborn paths drop even when the worktree is dirty. - Reset committed paths in the real index to `HEAD` after an isolated commit so hook-updated blobs are not left stale. - Cover dirty leftover isolation on an unborn branch and a pre-commit hook that restages a formatted file. --- src/infra/git/repo.ts | 15 +++++++- test/infra/git/repo.integration.test.ts | 50 ++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index f4e3c22..1e7db22 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -124,7 +124,7 @@ const resetIndexPaths = (root: string, indexFile: string, paths: readonly string : execBin("git", ["-C", root, "rev-parse", "-q", "--verify", "HEAD"]).chain((head) => execGitChecked( head.either( - () => ["-C", root, "rm", "--cached", "-q", "--", ...paths], + () => ["-C", root, "rm", "--cached", "-q", "-f", "--", ...paths], () => ["-C", root, "reset", "-q", "HEAD", "--", ...paths] ), "Failed to isolate staged paths", @@ -132,6 +132,15 @@ const resetIndexPaths = (root: string, indexFile: string, paths: readonly string ).map(() => {}) ); +const reconcileCommittedIndex = (root: string, paths: readonly string[]): Future => + execGitChecked(["-C", root, "reset", "-q", "HEAD", "--", ...paths], "Failed to reconcile index after commit").map(() => {}); + +const finishIsolatedCommit = (root: string, paths: readonly string[], result: ExecResult): Future => + result.either( + () => Future.resolve(result), + () => reconcileCommittedIndex(root, paths).map(() => result) + ); + const commitIsolatedPaths = (root: string, messageFile: string, paths: readonly string[]): Future => { const tmpIndex = join(tmpdir(), `commit-index-${Date.now()}`); return Future.bracket( @@ -144,7 +153,9 @@ const commitIsolatedPaths = (root: string, messageFile: string, paths: readonly root, tmpIndex, staged.filter((path) => !keep.has(path)) - ).chain(() => execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex))); + ).chain(() => + execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)).chain((result) => finishIsolatedCommit(root, paths, result)) + ); }) ); }; diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 4b7ab39..c697bf8 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { chdir, cwd } from "node:process"; -import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { execSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -182,6 +182,54 @@ describe("git repo integration", () => { } }); + it("performCommit with pathspecs isolates unborn leftovers when worktree is dirty", async () => { + const dir = mkdtempSync(join(tmpdir(), "commit-tools-git-")); + const run = (args: string) => execSync(`git ${args}`, { cwd: dir, encoding: "utf-8" }); + run("init -b main"); + run('config user.email "test@example.com"'); + run('config user.name "Test"'); + writeFileSync(join(dir, "a.txt"), "a\n"); + writeFileSync(join(dir, "b.txt"), "b\n"); + run("add a.txt b.txt"); + writeFileSync(join(dir, "b.txt"), "b dirty\n"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: first", ["a.txt"])); + expect(run("diff --staged --name-only").trim()).toBe("b.txt"); + expect(run("show :b.txt")).toBe("b\n"); + expect(run("show HEAD:a.txt")).toBe("a\n"); + } finally { + chdir(prev); + } + }); + + it("performCommit with pathspecs copies hook-updated blobs into the real index", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "file.txt"), "unformatted\n"); + writeFileSync(join(dir, "other.txt"), "other\n"); + run("add file.txt other.txt"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +printf 'formatted\\n' > file.txt +git add file.txt +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: formatted", ["file.txt"])); + expect(run("show HEAD:file.txt")).toBe("formatted\n"); + expect(run("show :file.txt")).toBe("formatted\n"); + expect(run("diff --staged --name-only").trim()).toBe("other.txt"); + } finally { + chdir(prev); + } + }); + it("performCommit pathspecs resolve from worktree root when cwd is a subdirectory", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "app")); From 945caf22dc9bf86d385efb7d99c1b719c52ffa2a Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:48:57 -0300 Subject: [PATCH 21/41] Keep later split groups out of hook restages - Run isolated `git commit` through a wrapper `pre-commit` that re-resets unselected paths after the user's hook. - Cover a formatter hook that `git add -A`s later groups so the first commit stays isolated. --- src/infra/git/repo.ts | 102 ++++++++++++++++++++---- test/infra/git/repo.integration.test.ts | 31 +++++++ 2 files changed, 119 insertions(+), 14 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 1e7db22..a6b0178 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -31,9 +31,10 @@ import { absurd } from "@/libs/types"; import { type BaseLookupError } from "@/infra/git/parsers"; import { execBin, type ExecResult } from "@/infra/shell"; import * as Decoder from "@/libs/json/decoder"; -import { copyFile, unlink, writeFile } from "node:fs/promises"; +import { constants as fsConstants } from "node:fs"; +import { access, copyFile, mkdir, readdir, rm, symlink, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { isAbsolute, join } from "node:path"; import { parsePushRange, formatCommitOutput, @@ -108,7 +109,81 @@ const createAndSwitchBranch = (name: string): Future => const getWorkTreeRoot = (): Future => execGitChecked(["rev-parse", "--show-toplevel"], "Failed to resolve git directory").map((s) => s.trim()); -const indexEnv = (indexFile: string): NodeJS.ProcessEnv => ({ GIT_INDEX_FILE: indexFile }); +const indexEnv = (indexFile: string, extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => ({ + GIT_INDEX_FILE: indexFile, + ...extra +}); + +const PRE_COMMIT_WRAPPER = `#!/bin/sh +set -e +if [ -n "$COMMIT_TOOLS_ORIG_PRE_COMMIT" ] && [ -x "$COMMIT_TOOLS_ORIG_PRE_COMMIT" ]; then + "$COMMIT_TOOLS_ORIG_PRE_COMMIT" +fi +if [ -z "$COMMIT_TOOLS_RESET_PATHS" ] || [ ! -s "$COMMIT_TOOLS_RESET_PATHS" ]; then + exit 0 +fi +if git rev-parse -q --verify HEAD >/dev/null; then + git reset -q HEAD --pathspec-from-file="$COMMIT_TOOLS_RESET_PATHS" --pathspec-file-nul || true +else + git rm --cached -q -f --pathspec-from-file="$COMMIT_TOOLS_RESET_PATHS" --pathspec-file-nul || true +fi +`; + +type HookIsolation = { hooksDir: string; pathsFile: string; origPreCommit: string }; + +const resolveHooksDir = (root: string): Future => + execGitChecked(["-C", root, "rev-parse", "--git-path", "hooks"], "Failed to resolve git hooks").map((p) => { + const trimmed = p.trim(); + return isAbsolute(trimmed) ? trimmed : join(root, trimmed); + }); + +const acquireHookIsolation = (root: string, unselected: readonly string[]): Future => { + const hooksDir = join(tmpdir(), `commit-hooks-${Date.now()}`); + const pathsFile = join(tmpdir(), `commit-reset-${Date.now()}`); + return resolveHooksDir(root).chain((origHooks) => + Future.attemptP(async () => { + await mkdir(hooksDir); + await writeFile(pathsFile, `${unselected.join("\0")}\0`); + await writeFile(join(hooksDir, "pre-commit"), PRE_COMMIT_WRAPPER, { mode: 0o755 }); + const names = await readdir(origHooks).catch(() => [] as string[]); + await Promise.all(names.filter((name) => name !== "pre-commit").map((name) => symlink(join(origHooks, name), join(hooksDir, name)))); + return { hooksDir, pathsFile, origPreCommit: join(origHooks, "pre-commit") }; + }) + ); +}; + +const releaseHookIsolation = (iso: HookIsolation): Future => + Future.attemptP(async () => { + await rm(iso.hooksDir, { recursive: true, force: true }); + await unlink(iso.pathsFile).catch(() => {}); + }); + +const hasExecutablePreCommit = (root: string): Future => + resolveHooksDir(root).chain((hooks) => + Future.attemptP(() => + access(join(hooks, "pre-commit"), fsConstants.X_OK) + .then(() => true) + .catch(() => false) + ) + ); + +const commitIsolatedIndex = (root: string, messageFile: string, tmpIndex: string, unselected: readonly string[]): Future => + unselected.length === 0 ? + execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)) + : hasExecutablePreCommit(root).chain((wrap) => + wrap ? + Future.bracket(acquireHookIsolation(root, unselected), releaseHookIsolation, (iso) => + execBin( + "git", + ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], + indexEnv(tmpIndex, { + COMMIT_TOOLS_ORIG_PRE_COMMIT: iso.origPreCommit, + COMMIT_TOOLS_RESET_PATHS: iso.pathsFile + }) + ) + ) + : execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)) + ); const copyIndexFile = (root: string, dest: string): Future => execGitChecked(["-C", root, "rev-parse", "--absolute-git-dir"], "Failed to resolve git directory").chain((gitDir) => @@ -141,22 +216,21 @@ const finishIsolatedCommit = (root: string, paths: readonly string[], result: Ex () => reconcileCommittedIndex(root, paths).map(() => result) ); +const isolateAndCommit = (root: string, messageFile: string, tmpIndex: string, paths: readonly string[]): Future => + listStagedPathsNoRenames(root).chain((staged) => { + const keep = new Set(paths); + const unselected = staged.filter((path) => !keep.has(path)); + return resetIndexPaths(root, tmpIndex, unselected).chain(() => + commitIsolatedIndex(root, messageFile, tmpIndex, unselected).chain((result) => finishIsolatedCommit(root, paths, result)) + ); + }); + const commitIsolatedPaths = (root: string, messageFile: string, paths: readonly string[]): Future => { const tmpIndex = join(tmpdir(), `commit-index-${Date.now()}`); return Future.bracket( copyIndexFile(root, tmpIndex), () => Future.attemptP(() => unlink(tmpIndex).catch(() => {})), - () => - listStagedPathsNoRenames(root).chain((staged) => { - const keep = new Set(paths); - return resetIndexPaths( - root, - tmpIndex, - staged.filter((path) => !keep.has(path)) - ).chain(() => - execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)).chain((result) => finishIsolatedCommit(root, paths, result)) - ); - }) + () => isolateAndCommit(root, messageFile, tmpIndex, paths) ); }; diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index c697bf8..b6f8841 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -204,6 +204,37 @@ describe("git repo integration", () => { } }); + it("performCommit with pathspecs ignores hook git add of later groups", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "a.txt"), "a\n"); + writeFileSync(join(dir, "b.txt"), "b\n"); + run("add a.txt b.txt"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +printf 'fmt-a\\n' > a.txt +printf 'fmt-b\\n' > b.txt +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: first", ["a.txt"])); + expect(run("show HEAD:a.txt")).toBe("fmt-a\n"); + expect(() => run("show HEAD:b.txt")).toThrow(); + expect(run("diff --staged --name-only").trim()).toBe("b.txt"); + expect(run("show :b.txt")).toBe("b\n"); + await runFuture(repo.performCommit("feat: second", ["b.txt"])); + expect(run("show HEAD:b.txt")).toBe("fmt-b\n"); + expect(run("diff --staged --name-only").trim()).toBe(""); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs copies hook-updated blobs into the real index", async () => { const { dir, run } = createTempGitRepo({ staged: false }); writeFileSync(join(dir, "file.txt"), "unformatted\n"); From fd5e01a5e9ae9abc079b019fdd48b9acecfba4cf Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:49:21 -0300 Subject: [PATCH 22/41] Reject isolated commits during in-progress merges - Fail pathspec isolation when `MERGE_HEAD`, `REBASE_HEAD`, `CHERRY_PICK_HEAD`, `REVERT_HEAD`, or unmerged index entries exist. - Keep ordinary `performCommit` without paths so a merge can finish as one commit. - Cover a conflicted merge so split isolation cannot consume `MERGE_HEAD`. --- src/infra/git/repo.ts | 34 ++++++++++++++++++++++--- test/infra/git/repo.integration.test.ts | 30 ++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index a6b0178..24da10b 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -225,12 +225,38 @@ const isolateAndCommit = (root: string, messageFile: string, tmpIndex: string, p ); }); +const SEQUENCER_REFS = ["MERGE_HEAD", "REBASE_HEAD", "CHERRY_PICK_HEAD", "REVERT_HEAD"] as const; + +const refExists = (root: string, ref: string): Future => + execBin("git", ["-C", root, "rev-parse", "-q", "--verify", ref]).map((result) => + result.either( + () => false, + () => true + ) + ); + +const rejectIfUnmergedPaths = (root: string): Future => + execGitChecked(["-C", root, "ls-files", "-u", "-z"], "Failed to list unmerged files").chain((stdout) => + splitNulPaths(stdout).length === 0 ? + Future.resolve(undefined) + : Future.reject(new Error("Cannot isolate a commit while the index has unmerged paths")) + ); + +const rejectIfSequencerInProgress = (root: string): Future => + Future.traverse((ref) => refExists(root, ref), [...SEQUENCER_REFS]).chain((present) => + present.some(Boolean) ? + Future.reject(new Error("Cannot isolate a commit while a merge, rebase, cherry-pick, or revert is in progress")) + : rejectIfUnmergedPaths(root) + ); + const commitIsolatedPaths = (root: string, messageFile: string, paths: readonly string[]): Future => { const tmpIndex = join(tmpdir(), `commit-index-${Date.now()}`); - return Future.bracket( - copyIndexFile(root, tmpIndex), - () => Future.attemptP(() => unlink(tmpIndex).catch(() => {})), - () => isolateAndCommit(root, messageFile, tmpIndex, paths) + return rejectIfSequencerInProgress(root).chain(() => + Future.bracket( + copyIndexFile(root, tmpIndex), + () => Future.attemptP(() => unlink(tmpIndex).catch(() => {})), + () => isolateAndCommit(root, messageFile, tmpIndex, paths) + ) ); }; diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index b6f8841..634d3f1 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -261,6 +261,36 @@ git add file.txt } }); + it("performCommit with pathspecs rejects during a merge", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "a.txt"), "a\n"); + writeFileSync(join(dir, "b.txt"), "b\n"); + run("add a.txt b.txt"); + run('commit -m "base"'); + run("checkout -b other"); + writeFileSync(join(dir, "b.txt"), "other-b\n"); + run("add b.txt"); + run('commit -m "other"'); + run("checkout main"); + writeFileSync(join(dir, "a.txt"), "main-a\n"); + writeFileSync(join(dir, "b.txt"), "main-b\n"); + run("add a.txt b.txt"); + run('commit -m "main"'); + try { + run("merge other"); + } catch { + // conflict on b.txt + } + const prev = cwd(); + chdir(dir); + try { + await expect(runFuture(repo.performCommit("feat: split merge", ["a.txt"]))).rejects.toThrow(/merge, rebase, cherry-pick, or revert/); + expect(run("rev-parse -q --verify MERGE_HEAD").trim().length).toBeGreaterThan(0); + } finally { + chdir(prev); + } + }); + it("performCommit pathspecs resolve from worktree root when cwd is a subdirectory", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "app")); From 6db2fcba12969f3e2d39e797797c3bca87dab9a8 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 21:59:49 -0300 Subject: [PATCH 23/41] Hide unstaged selected edits while hooks run - Check out staged blobs into the worktree before an isolated commit when a `pre-commit` hook exists. - Restore the original worktree afterward so a hook `git add -A` cannot commit unstaged selected content. - Cover a selected file staged as safe then edited to a secret before `git add -A`. --- src/infra/git/repo.ts | 94 ++++++++++++++++++++----- test/infra/git/repo.integration.test.ts | 26 +++++++ 2 files changed, 103 insertions(+), 17 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 24da10b..7ad3ebb 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -34,7 +34,7 @@ import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; import { access, copyFile, mkdir, readdir, rm, symlink, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { isAbsolute, join } from "node:path"; +import { dirname, isAbsolute, join } from "node:path"; import { parsePushRange, formatCommitOutput, @@ -167,23 +167,83 @@ const hasExecutablePreCommit = (root: string): Future => ) ); -const commitIsolatedIndex = (root: string, messageFile: string, tmpIndex: string, unselected: readonly string[]): Future => - unselected.length === 0 ? - execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)) - : hasExecutablePreCommit(root).chain((wrap) => - wrap ? - Future.bracket(acquireHookIsolation(root, unselected), releaseHookIsolation, (iso) => - execBin( - "git", - ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], - indexEnv(tmpIndex, { - COMMIT_TOOLS_ORIG_PRE_COMMIT: iso.origPreCommit, - COMMIT_TOOLS_RESET_PATHS: iso.pathsFile - }) - ) +type WorktreeSnapshot = { dir: string; paths: readonly string[] }; + +const backupWorktreeFiles = (root: string, dir: string, paths: readonly string[]): Promise => + Promise.all( + paths.map(async (rel) => { + const dest = join(dir, rel); + await mkdir(dirname(dest), { recursive: true }); + await copyFile(join(root, rel), dest).catch(() => {}); + }) + ).then(() => {}); + +const acquireWorktreeSnapshot = (root: string, tmpIndex: string, selected: readonly string[]): Future => { + const dir = join(tmpdir(), `commit-wt-${Date.now()}`); + const snap: WorktreeSnapshot = { dir, paths: selected }; + return Future.attemptP(async () => { + await mkdir(dir); + await backupWorktreeFiles(root, dir, selected); + }) + .chain(() => + selected.length === 0 ? + Future.resolve(snap) + : execGitChecked(["-C", root, "checkout-index", "-f", "--", ...selected], "Failed to isolate worktree for hooks", indexEnv(tmpIndex)).map( + () => snap ) - : execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)) + ) + .chainRej((err) => Future.attemptP(() => rm(dir, { recursive: true, force: true })).chain(() => Future.reject(err))); +}; + +const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future => + Future.attemptP(async () => { + await Promise.all( + snap.paths.map(async (rel) => { + const backup = join(snap.dir, rel); + const dest = join(root, rel); + try { + await access(backup); + await mkdir(dirname(dest), { recursive: true }); + await copyFile(backup, dest); + } catch { + await unlink(dest).catch(() => {}); + } + }) ); + await rm(snap.dir, { recursive: true, force: true }); + }); + +const runIsolatedCommit = ( + root: string, + messageFile: string, + tmpIndex: string, + unselected: readonly string[], + wrapHooks: boolean +): Future => + wrapHooks ? + Future.bracket(acquireHookIsolation(root, unselected), releaseHookIsolation, (iso) => + execBin( + "git", + ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], + indexEnv(tmpIndex, { + COMMIT_TOOLS_ORIG_PRE_COMMIT: iso.origPreCommit, + COMMIT_TOOLS_RESET_PATHS: iso.pathsFile + }) + ) + ) + : execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)); + +const commitIsolatedIndex = ( + root: string, + messageFile: string, + tmpIndex: string, + selected: readonly string[], + unselected: readonly string[] +): Future => + hasExecutablePreCommit(root).chain((hasHook) => { + const commit = () => runIsolatedCommit(root, messageFile, tmpIndex, unselected, hasHook && unselected.length > 0); + return hasHook ? Future.bracket(acquireWorktreeSnapshot(root, tmpIndex, selected), (snap) => releaseWorktreeSnapshot(root, snap), commit) : commit(); + }); const copyIndexFile = (root: string, dest: string): Future => execGitChecked(["-C", root, "rev-parse", "--absolute-git-dir"], "Failed to resolve git directory").chain((gitDir) => @@ -221,7 +281,7 @@ const isolateAndCommit = (root: string, messageFile: string, tmpIndex: string, p const keep = new Set(paths); const unselected = staged.filter((path) => !keep.has(path)); return resetIndexPaths(root, tmpIndex, unselected).chain(() => - commitIsolatedIndex(root, messageFile, tmpIndex, unselected).chain((result) => finishIsolatedCommit(root, paths, result)) + commitIsolatedIndex(root, messageFile, tmpIndex, paths, unselected).chain((result) => finishIsolatedCommit(root, paths, result)) ); }); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 634d3f1..d514642 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -235,6 +235,32 @@ git add -A } }); + it("performCommit with pathspecs does not commit unstaged selected edits via hook git add -A", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "a.txt"), "staged safe\n"); + writeFileSync(join(dir, "b.txt"), "b\n"); + run("add a.txt b.txt"); + writeFileSync(join(dir, "a.txt"), "unstaged SECRET\n"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: first", ["a.txt"])); + expect(run("show HEAD:a.txt")).toBe("staged safe\n"); + expect(run("diff -- a.txt")).toContain("unstaged SECRET"); + expect(() => run("show HEAD:b.txt")).toThrow(); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs copies hook-updated blobs into the real index", async () => { const { dir, run } = createTempGitRepo({ staged: false }); writeFileSync(join(dir, "file.txt"), "unformatted\n"); From 10fc7efef03145ce20122f6cd6b0f982c9bfbdae Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 22:10:34 -0300 Subject: [PATCH 24/41] Snapshot only dirty index paths for hook isolation - Skip `checkout-index` for staged deletions and restore only paths that had unstaged edits. - Restore the backup if snapshot acquire fails so a partial checkout cannot drop worktree files. - Cover a rename plus `git add -A` hook, and keep formatter worktree updates when there were no unstaged edits. --- src/infra/git/repo.ts | 62 +++++++++++++++++++------ test/infra/git/repo.integration.test.ts | 29 +++++++++++- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 7ad3ebb..63ab8d8 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -167,7 +167,29 @@ const hasExecutablePreCommit = (root: string): Future => ) ); -type WorktreeSnapshot = { dir: string; paths: readonly string[] }; +type WorktreeSnapshot = { dir: string; paths: readonly string[]; checkout: readonly string[] }; + +const nulPathSet = (stdout: string): ReadonlySet => new Set(splitNulPaths(stdout)); + +const indexPathSet = (root: string, tmpIndex: string, paths: readonly string[]): Future> => + paths.length === 0 ? + Future.resolve(new Set()) + : execGitChecked(["-C", root, "ls-files", "-z", "--", ...paths], "Failed to list index paths", indexEnv(tmpIndex)).map(nulPathSet); + +const dirtyPathSet = (root: string, tmpIndex: string, paths: readonly string[]): Future> => + paths.length === 0 ? + Future.resolve(new Set()) + : execGitChecked(["-C", root, "diff", "--name-only", "-z", "--", ...paths], "Failed to list dirty worktree paths", indexEnv(tmpIndex)).map(nulPathSet); + +const planWorktreeHide = ( + selected: readonly string[], + inIndex: ReadonlySet, + dirty: ReadonlySet +): { checkout: readonly string[]; hide: readonly string[] } => { + const checkout = selected.filter((path) => inIndex.has(path) && dirty.has(path)); + const deleted = selected.filter((path) => !inIndex.has(path)); + return { checkout, hide: [...checkout, ...deleted] }; +}; const backupWorktreeFiles = (root: string, dir: string, paths: readonly string[]): Promise => Promise.all( @@ -180,23 +202,35 @@ const backupWorktreeFiles = (root: string, dir: string, paths: readonly string[] const acquireWorktreeSnapshot = (root: string, tmpIndex: string, selected: readonly string[]): Future => { const dir = join(tmpdir(), `commit-wt-${Date.now()}`); - const snap: WorktreeSnapshot = { dir, paths: selected }; - return Future.attemptP(async () => { - await mkdir(dir); - await backupWorktreeFiles(root, dir, selected); - }) - .chain(() => - selected.length === 0 ? - Future.resolve(snap) - : execGitChecked(["-C", root, "checkout-index", "-f", "--", ...selected], "Failed to isolate worktree for hooks", indexEnv(tmpIndex)).map( - () => snap - ) - ) - .chainRej((err) => Future.attemptP(() => rm(dir, { recursive: true, force: true })).chain(() => Future.reject(err))); + return Future.both(indexPathSet(root, tmpIndex, selected), dirtyPathSet(root, tmpIndex, selected)).chain(([inIndex, dirty]) => { + const { checkout, hide } = planWorktreeHide(selected, inIndex, dirty); + const snap: WorktreeSnapshot = { dir, paths: hide, checkout }; + if (hide.length === 0) { + return Future.resolve(snap); + } + return Future.attemptP(async () => { + await mkdir(dir); + await backupWorktreeFiles(root, dir, hide); + await Promise.all(hide.filter((path) => !checkout.includes(path)).map((rel) => unlink(join(root, rel)).catch(() => {}))); + }) + .chain(() => + checkout.length === 0 ? + Future.resolve(snap) + : execGitChecked(["-C", root, "checkout-index", "-f", "--", ...checkout], "Failed to isolate worktree for hooks", indexEnv(tmpIndex)).map( + () => snap + ) + ) + .chainRej((err) => releaseWorktreeSnapshot(root, snap).chain(() => Future.reject(err))); + }); }; const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future => Future.attemptP(async () => { + try { + await access(snap.dir); + } catch { + return; + } await Promise.all( snap.paths.map(async (rel) => { const backup = join(snap.dir, rel); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index d514642..8a7706f 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { chdir, cwd } from "node:process"; -import { chmodSync, mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; import { execSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -144,6 +144,32 @@ describe("git repo integration", () => { } }); + it("performCommit with pathspecs isolates a rename when a hook runs git add -A", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "other.txt"), "other\n"); + run("add other.txt"); + run("mv file.txt renamed.txt"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + const staged = await runFuture(repo.listStagedPaths()); + expect([...staged].sort()).toEqual(["file.txt", "other.txt", "renamed.txt"]); + await runFuture(repo.performCommit("feat: rename file", ["file.txt", "renamed.txt"])); + expect(run("diff --staged --name-only").trim()).toBe("other.txt"); + expect(run("ls-files").trim().split("\n").sort()).toEqual(["other.txt", "renamed.txt"]); + } finally { + chdir(prev); + } + }); + it("performCommit with paths records the staged blob not the worktree", async () => { const { dir, run } = createTempGitRepo({ staged: true }); writeFileSync(join(dir, "file.txt"), "hello unstaged secret\n"); @@ -281,6 +307,7 @@ git add file.txt await runFuture(repo.performCommit("feat: formatted", ["file.txt"])); expect(run("show HEAD:file.txt")).toBe("formatted\n"); expect(run("show :file.txt")).toBe("formatted\n"); + expect(readFileSync(join(dir, "file.txt"), "utf-8")).toBe("formatted\n"); expect(run("diff --staged --name-only").trim()).toBe("other.txt"); } finally { chdir(prev); From 8f85fe73e2d9c4fa79dc54add956737e09e0b50b Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 22:15:12 -0300 Subject: [PATCH 25/41] Harden hook snapshots and literal pathspecs - Snapshot and restore worktree symlinks as links, and fail backup unless the source is missing (`ENOENT`). - Set `GIT_LITERAL_PATHSPECS` on git invocations so names like `app/[id]/page.tsx` do not match other paths. - Cover symlink retarget restore and a bracket-filename split isolation. --- src/infra/git/repo.ts | 44 ++++++++++++++++++----- test/infra/git/repo.integration.test.ts | 48 ++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 63ab8d8..fc66cee 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -32,7 +32,7 @@ import { type BaseLookupError } from "@/infra/git/parsers"; import { execBin, type ExecResult } from "@/infra/shell"; import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; -import { access, copyFile, mkdir, readdir, rm, symlink, unlink, writeFile } from "node:fs/promises"; +import { access, copyFile, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; import { @@ -61,7 +61,7 @@ type PushResult = { }; const execGitChecked = (args: string[], fallbackMsg: string, env?: NodeJS.ProcessEnv): Future => - execBin("git", args, env).chain((result) => + execBin("git", args, { GIT_LITERAL_PATHSPECS: "1", ...env }).chain((result) => result.either( (failure) => Future.reject(new Error(commandFailureMessage(failure, fallbackMsg))), ({ stdout }) => Future.resolve(stdout) @@ -111,6 +111,7 @@ const getWorkTreeRoot = (): Future => const indexEnv = (indexFile: string, extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => ({ GIT_INDEX_FILE: indexFile, + GIT_LITERAL_PATHSPECS: "1", ...extra }); @@ -191,12 +192,39 @@ const planWorktreeHide = ( return { checkout, hide: [...checkout, ...deleted] }; }; +const isEnoent = (err: unknown): boolean => err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT"; + +const snapshotWorktreeEntry = async (src: string, dest: string): Promise => { + const st = await lstat(src); + await mkdir(dirname(dest), { recursive: true }); + if (st.isSymbolicLink()) { + await symlink(await readlink(src), dest); + return; + } + await copyFile(src, dest); +}; + +const restoreWorktreeEntry = async (backup: string, dest: string): Promise => { + const st = await lstat(backup); + await unlink(dest).catch(() => {}); + await mkdir(dirname(dest), { recursive: true }); + if (st.isSymbolicLink()) { + await symlink(await readlink(backup), dest); + return; + } + await copyFile(backup, dest); +}; + const backupWorktreeFiles = (root: string, dir: string, paths: readonly string[]): Promise => Promise.all( paths.map(async (rel) => { - const dest = join(dir, rel); - await mkdir(dirname(dest), { recursive: true }); - await copyFile(join(root, rel), dest).catch(() => {}); + try { + await snapshotWorktreeEntry(join(root, rel), join(dir, rel)); + } catch (err) { + if (!isEnoent(err)) { + throw err; + } + } }) ).then(() => {}); @@ -236,12 +264,12 @@ const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future {}); + return; } + await restoreWorktreeEntry(backup, dest); }) ); await rm(snap.dir, { recursive: true, force: true }); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 8a7706f..2d30ad3 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { chdir, cwd } from "node:process"; -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, symlinkSync, unlinkSync, writeFileSync } from "node:fs"; import { execSync } from "node:child_process"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -287,6 +287,52 @@ git add -A } }); + it("performCommit with pathspecs restores an unstaged symlink retarget after a hook", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "target.txt"), "staged-target\n"); + writeFileSync(join(dir, "link-dest.txt"), "unstaged-dest\n"); + symlinkSync("target.txt", join(dir, "link")); + run("add link"); + unlinkSync(join(dir, "link")); + symlinkSync("link-dest.txt", join(dir, "link")); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: link", ["link"])); + expect(run("show HEAD:link").trim()).toBe("target.txt"); + expect(readlinkSync(join(dir, "link"))).toBe("link-dest.txt"); + expect(readFileSync(join(dir, "target.txt"), "utf-8")).toBe("staged-target\n"); + } finally { + chdir(prev); + } + }); + + it("performCommit with pathspecs treats bracket filenames as literal pathspecs", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + mkdirSync(join(dir, "app", "[id]"), { recursive: true }); + mkdirSync(join(dir, "app", "i"), { recursive: true }); + writeFileSync(join(dir, "app", "[id]", "page.tsx"), "bracket\n"); + writeFileSync(join(dir, "app", "i", "page.tsx"), "plain\n"); + run("add app"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: bracket route", ["app/[id]/page.tsx"])); + expect(run("show HEAD:app/[id]/page.tsx")).toBe("bracket\n"); + expect(run("diff --staged --name-only").trim()).toBe("app/i/page.tsx"); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs copies hook-updated blobs into the real index", async () => { const { dir, run } = createTempGitRepo({ staged: false }); writeFileSync(join(dir, "file.txt"), "unformatted\n"); From 0f702af6f4520cb0bfa486ce00818ba36a91217a Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 22:31:15 -0300 Subject: [PATCH 26/41] Run isolated pre-commit hooks outside git commit - Invoke the user `pre-commit` from Node, then reset every non-selected tmp-index path so `git add -A` cannot commit untracked files. - Keep `GIT_LITERAL_PATHSPECS` off the commit environment so hooks still see glob pathspecs. - Copy hook files instead of symlinking them, and snapshot directories through file/dir transitions. - Cover untracked hook restage, glob hooks, and file-to-directory replacements. --- src/infra/git/repo.ts | 134 ++++++++++++++---------- src/infra/shell.ts | 3 +- test/infra/git/repo.integration.test.ts | 94 +++++++++++++++++ 3 files changed, 173 insertions(+), 58 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index fc66cee..544c89a 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -32,7 +32,7 @@ import { type BaseLookupError } from "@/infra/git/parsers"; import { execBin, type ExecResult } from "@/infra/shell"; import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; -import { access, copyFile, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; +import { access, copyFile, cp, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; import { @@ -111,26 +111,10 @@ const getWorkTreeRoot = (): Future => const indexEnv = (indexFile: string, extra: NodeJS.ProcessEnv = {}): NodeJS.ProcessEnv => ({ GIT_INDEX_FILE: indexFile, - GIT_LITERAL_PATHSPECS: "1", ...extra }); -const PRE_COMMIT_WRAPPER = `#!/bin/sh -set -e -if [ -n "$COMMIT_TOOLS_ORIG_PRE_COMMIT" ] && [ -x "$COMMIT_TOOLS_ORIG_PRE_COMMIT" ]; then - "$COMMIT_TOOLS_ORIG_PRE_COMMIT" -fi -if [ -z "$COMMIT_TOOLS_RESET_PATHS" ] || [ ! -s "$COMMIT_TOOLS_RESET_PATHS" ]; then - exit 0 -fi -if git rev-parse -q --verify HEAD >/dev/null; then - git reset -q HEAD --pathspec-from-file="$COMMIT_TOOLS_RESET_PATHS" --pathspec-file-nul || true -else - git rm --cached -q -f --pathspec-from-file="$COMMIT_TOOLS_RESET_PATHS" --pathspec-file-nul || true -fi -`; - -type HookIsolation = { hooksDir: string; pathsFile: string; origPreCommit: string }; +type HookIsolation = { hooksDir: string; origPreCommit: string }; const resolveHooksDir = (root: string): Future => execGitChecked(["-C", root, "rev-parse", "--git-path", "hooks"], "Failed to resolve git hooks").map((p) => { @@ -138,17 +122,23 @@ const resolveHooksDir = (root: string): Future => return isAbsolute(trimmed) ? trimmed : join(root, trimmed); }); -const acquireHookIsolation = (root: string, unselected: readonly string[]): Future => { +const copyHookEntry = async (src: string, dest: string): Promise => { + const st = await lstat(src); + if (st.isDirectory()) { + await cp(src, dest, { recursive: true }); + return; + } + await copyFile(src, dest); +}; + +const acquireHookIsolation = (root: string): Future => { const hooksDir = join(tmpdir(), `commit-hooks-${Date.now()}`); - const pathsFile = join(tmpdir(), `commit-reset-${Date.now()}`); return resolveHooksDir(root).chain((origHooks) => Future.attemptP(async () => { await mkdir(hooksDir); - await writeFile(pathsFile, `${unselected.join("\0")}\0`); - await writeFile(join(hooksDir, "pre-commit"), PRE_COMMIT_WRAPPER, { mode: 0o755 }); const names = await readdir(origHooks).catch(() => [] as string[]); - await Promise.all(names.filter((name) => name !== "pre-commit").map((name) => symlink(join(origHooks, name), join(hooksDir, name)))); - return { hooksDir, pathsFile, origPreCommit: join(origHooks, "pre-commit") }; + await Promise.all(names.filter((name) => name !== "pre-commit").map((name) => copyHookEntry(join(origHooks, name), join(hooksDir, name)))); + return { hooksDir, origPreCommit: join(origHooks, "pre-commit") }; }) ); }; @@ -156,7 +146,6 @@ const acquireHookIsolation = (root: string, unselected: readonly string[]): Futu const releaseHookIsolation = (iso: HookIsolation): Future => Future.attemptP(async () => { await rm(iso.hooksDir, { recursive: true, force: true }); - await unlink(iso.pathsFile).catch(() => {}); }); const hasExecutablePreCommit = (root: string): Future => @@ -192,7 +181,29 @@ const planWorktreeHide = ( return { checkout, hide: [...checkout, ...deleted] }; }; -const isEnoent = (err: unknown): boolean => err instanceof Error && "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT"; +const errnoCode = (err: unknown): string | undefined => (err instanceof Error && "code" in err ? (err as NodeJS.ErrnoException).code : undefined); + +const isStructuralPathErr = (err: unknown): boolean => { + const code = errnoCode(err); + return code === "ENOENT" || code === "ENOTDIR" || code === "EISDIR"; +}; + +const removeWorktreePath = async (path: string): Promise => { + let st; + try { + st = await lstat(path); + } catch (err) { + if (isStructuralPathErr(err)) { + return; + } + throw err; + } + if (st.isDirectory()) { + await rm(path, { recursive: true, force: true }); + return; + } + await unlink(path); +}; const snapshotWorktreeEntry = async (src: string, dest: string): Promise => { const st = await lstat(src); @@ -201,17 +212,25 @@ const snapshotWorktreeEntry = async (src: string, dest: string): Promise = await symlink(await readlink(src), dest); return; } + if (st.isDirectory()) { + await cp(src, dest, { recursive: true }); + return; + } await copyFile(src, dest); }; const restoreWorktreeEntry = async (backup: string, dest: string): Promise => { const st = await lstat(backup); - await unlink(dest).catch(() => {}); + await removeWorktreePath(dest); await mkdir(dirname(dest), { recursive: true }); if (st.isSymbolicLink()) { await symlink(await readlink(backup), dest); return; } + if (st.isDirectory()) { + await cp(backup, dest, { recursive: true }); + return; + } await copyFile(backup, dest); }; @@ -221,7 +240,7 @@ const backupWorktreeFiles = (root: string, dir: string, paths: readonly string[] try { await snapshotWorktreeEntry(join(root, rel), join(dir, rel)); } catch (err) { - if (!isEnoent(err)) { + if (!isStructuralPathErr(err)) { throw err; } } @@ -239,7 +258,7 @@ const acquireWorktreeSnapshot = (root: string, tmpIndex: string, selected: reado return Future.attemptP(async () => { await mkdir(dir); await backupWorktreeFiles(root, dir, hide); - await Promise.all(hide.filter((path) => !checkout.includes(path)).map((rel) => unlink(join(root, rel)).catch(() => {}))); + await Promise.all(hide.filter((path) => !checkout.includes(path)).map((rel) => removeWorktreePath(join(root, rel)))); }) .chain(() => checkout.length === 0 ? @@ -266,7 +285,7 @@ const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future {}); + await removeWorktreePath(dest); return; } await restoreWorktreeEntry(backup, dest); @@ -275,35 +294,36 @@ const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future => - wrapHooks ? - Future.bracket(acquireHookIsolation(root, unselected), releaseHookIsolation, (iso) => - execBin( - "git", - ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], - indexEnv(tmpIndex, { - COMMIT_TOOLS_ORIG_PRE_COMMIT: iso.origPreCommit, - COMMIT_TOOLS_RESET_PATHS: iso.pathsFile - }) - ) +const runUserPreCommit = (root: string, tmpIndex: string): Future => + resolveHooksDir(root).chain((hooks) => execBin(join(hooks, "pre-commit"), [], indexEnv(tmpIndex), root)); + +const resetForeignIndexPaths = (root: string, tmpIndex: string, selected: readonly string[]): Future => + execGitChecked(["-C", root, "ls-files", "-z"], "Failed to list index after hook", indexEnv(tmpIndex)).chain((stdout) => { + const keep = new Set(selected); + return resetIndexPaths( + root, + tmpIndex, + splitNulPaths(stdout).filter((path) => !keep.has(path)) + ); + }); + +const commitAfterHook = (root: string, messageFile: string, tmpIndex: string, selected: readonly string[]): Future => + runUserPreCommit(root, tmpIndex).chain((hookResult) => + hookResult.either( + () => Future.resolve(hookResult), + () => + resetForeignIndexPaths(root, tmpIndex, selected).chain(() => + Future.bracket(acquireHookIsolation(root), releaseHookIsolation, (iso) => + execBin("git", ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], indexEnv(tmpIndex)) + ) + ) ) - : execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)); + ); -const commitIsolatedIndex = ( - root: string, - messageFile: string, - tmpIndex: string, - selected: readonly string[], - unselected: readonly string[] -): Future => +const commitIsolatedIndex = (root: string, messageFile: string, tmpIndex: string, selected: readonly string[]): Future => hasExecutablePreCommit(root).chain((hasHook) => { - const commit = () => runIsolatedCommit(root, messageFile, tmpIndex, unselected, hasHook && unselected.length > 0); + const commit = () => + hasHook ? commitAfterHook(root, messageFile, tmpIndex, selected) : execBin("git", ["-C", root, "commit", "-F", messageFile], indexEnv(tmpIndex)); return hasHook ? Future.bracket(acquireWorktreeSnapshot(root, tmpIndex, selected), (snap) => releaseWorktreeSnapshot(root, snap), commit) : commit(); }); @@ -343,7 +363,7 @@ const isolateAndCommit = (root: string, messageFile: string, tmpIndex: string, p const keep = new Set(paths); const unselected = staged.filter((path) => !keep.has(path)); return resetIndexPaths(root, tmpIndex, unselected).chain(() => - commitIsolatedIndex(root, messageFile, tmpIndex, paths, unselected).chain((result) => finishIsolatedCommit(root, paths, result)) + commitIsolatedIndex(root, messageFile, tmpIndex, paths).chain((result) => finishIsolatedCommit(root, paths, result)) ); }); diff --git a/src/infra/shell.ts b/src/infra/shell.ts index 05a0b7c..3a6f7c8 100644 --- a/src/infra/shell.ts +++ b/src/infra/shell.ts @@ -16,10 +16,11 @@ const commandResult = (output: CommandOutput, exitCode: number | null, signal: N Success(output) : Failure({ output, error: exitCodeError(exitCode, signal) }); -const execBin = (bin: string, args: string[], env?: NodeJS.ProcessEnv): Future => +const execBin = (bin: string, args: string[], env?: NodeJS.ProcessEnv, cwd?: string): Future => Future.create((reject, resolve) => { const proc = spawn(bin, args, { stdio: ["pipe", "pipe", "pipe"], + cwd, env: env === undefined ? undefined : { ...process.env, ...env } }); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 2d30ad3..46d0575 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -315,6 +315,100 @@ git add -A } }); + it("performCommit with pathspecs ignores untracked files added by hook git add -A", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "selected.txt"), "keep\n"); + writeFileSync(join(dir, "secret.txt"), "UNTRACKED SECRET\n"); + run("add selected.txt"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: selected", ["selected.txt"])); + expect(run("show HEAD:selected.txt")).toBe("keep\n"); + expect(() => run("show HEAD:secret.txt")).toThrow(); + expect(run("status --porcelain").trim()).toBe("?? secret.txt"); + } finally { + chdir(prev); + } + }); + + it("performCommit with pathspecs lets hooks see glob pathspecs", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "a.txt"), "a\n"); + run("add a.txt"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +files=$(git diff --cached --name-only -- '*.txt') +test -n "$files" +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: glob hook", ["a.txt"])); + expect(run("show HEAD:a.txt")).toBe("a\n"); + } finally { + chdir(prev); + } + }); + + it("performCommit with pathspecs isolates a file-to-directory replacement when a hook exists", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "thing"), "file\n"); + run("add thing"); + run('commit -m "add thing"'); + unlinkSync(join(dir, "thing")); + mkdirSync(join(dir, "thing")); + writeFileSync(join(dir, "thing", "child"), "child\n"); + run("add -A"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync(hookPath, "#!/bin/sh\nexit 0\n"); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: replace file", ["thing", "thing/child"])); + expect(run("show HEAD:thing/child")).toBe("child\n"); + expect(run("cat-file -t HEAD:thing").trim()).toBe("tree"); + } finally { + chdir(prev); + } + }); + + it("performCommit with pathspecs isolates a directory-to-file replacement when a hook exists", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + mkdirSync(join(dir, "thing")); + writeFileSync(join(dir, "thing", "child"), "child\n"); + run("add thing"); + run('commit -m "add dir"'); + run("rm -rf thing"); + writeFileSync(join(dir, "thing"), "file\n"); + run("add -A"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync(hookPath, "#!/bin/sh\nexit 0\n"); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + const staged = await runFuture(repo.listStagedPaths()); + await runFuture(repo.performCommit("feat: replace dir", staged)); + expect(run("show HEAD:thing")).toBe("file\n"); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs treats bracket filenames as literal pathspecs", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "app", "[id]"), { recursive: true }); From 997055cdd385fdb64abcf918f33edef4b01876c1 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 22:42:09 -0300 Subject: [PATCH 27/41] Keep file-to-directory replacements visible to hooks - Skip hiding a deleted selected path when the isolated index still has files under that path. - Cover a `git add -A` pre-commit hook on a staged file-to-directory replacement. --- src/infra/git/repo.ts | 4 +++- test/infra/git/repo.integration.test.ts | 30 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 544c89a..312bb83 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -171,13 +171,15 @@ const dirtyPathSet = (root: string, tmpIndex: string, paths: readonly string[]): Future.resolve(new Set()) : execGitChecked(["-C", root, "diff", "--name-only", "-z", "--", ...paths], "Failed to list dirty worktree paths", indexEnv(tmpIndex)).map(nulPathSet); +const hasIndexedDescendant = (path: string, inIndex: ReadonlySet): boolean => [...inIndex].some((indexed) => indexed.startsWith(`${path}/`)); + const planWorktreeHide = ( selected: readonly string[], inIndex: ReadonlySet, dirty: ReadonlySet ): { checkout: readonly string[]; hide: readonly string[] } => { const checkout = selected.filter((path) => inIndex.has(path) && dirty.has(path)); - const deleted = selected.filter((path) => !inIndex.has(path)); + const deleted = selected.filter((path) => !inIndex.has(path) && !hasIndexedDescendant(path, inIndex)); return { checkout, hide: [...checkout, ...deleted] }; }; diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 46d0575..8b34cef 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -386,6 +386,36 @@ test -n "$files" } }); + it("performCommit with pathspecs isolates a file-to-directory replacement when a hook runs git add -A", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "thing"), "file\n"); + run("add thing"); + run('commit -m "add thing"'); + unlinkSync(join(dir, "thing")); + mkdirSync(join(dir, "thing")); + writeFileSync(join(dir, "thing", "child"), "child\n"); + writeFileSync(join(dir, "other.txt"), "other\n"); + run("add -A"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: replace file", ["thing", "thing/child"])); + expect(run("show HEAD:thing/child")).toBe("child\n"); + expect(run("cat-file -t HEAD:thing").trim()).toBe("tree"); + expect(run("diff --staged --name-only").trim()).toBe("other.txt"); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs isolates a directory-to-file replacement when a hook exists", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "thing")); From 4a738fc5b33b137da28319d6d83cb88733816092 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 22:42:13 -0300 Subject: [PATCH 28/41] Reset hook-foreign index paths from a NUL pathspec file - Pass `resetIndexPaths` paths through `--pathspec-from-file` so large indexes do not exceed `ARG_MAX`. --- src/infra/git/repo.ts | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 312bb83..971a180 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -337,18 +337,30 @@ const copyIndexFile = (root: string, dest: string): Future => const listStagedPathsNoRenames = (root: string): Future => execGitChecked(["-C", root, "diff", "--staged", "--name-only", "--no-renames", "-z"], "Failed to list staged files").map(splitNulPaths); +const writeNulPathspecFile = (paths: readonly string[]): Future => + Future.attemptP(async () => { + const file = join(tmpdir(), `commit-pathspec-${Date.now()}`); + await writeFile(file, `${paths.join("\0")}\0`); + return file; + }); + const resetIndexPaths = (root: string, indexFile: string, paths: readonly string[]): Future => paths.length === 0 ? Future.resolve(undefined) : execBin("git", ["-C", root, "rev-parse", "-q", "--verify", "HEAD"]).chain((head) => - execGitChecked( - head.either( - () => ["-C", root, "rm", "--cached", "-q", "-f", "--", ...paths], - () => ["-C", root, "reset", "-q", "HEAD", "--", ...paths] - ), - "Failed to isolate staged paths", - indexEnv(indexFile) - ).map(() => {}) + Future.bracket( + writeNulPathspecFile(paths), + (file) => Future.attemptP(() => unlink(file).catch(() => {})), + (file) => + execGitChecked( + head.either( + () => ["-C", root, "rm", "--cached", "-q", "-f", `--pathspec-from-file=${file}`, "--pathspec-file-nul"], + () => ["-C", root, "reset", "-q", "HEAD", `--pathspec-from-file=${file}`, "--pathspec-file-nul"] + ), + "Failed to isolate staged paths", + indexEnv(indexFile) + ).map(() => {}) + ) ); const reconcileCommittedIndex = (root: string, paths: readonly string[]): Future => From fe57e84c0ba79d55a9a9f0bde1143188d8d9d447 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 22:53:56 -0300 Subject: [PATCH 29/41] Restore hook-foreign deletions after isolated pre-commit - Snapshot temporary-index paths before the hook and reset every nonselected path from the pre/post union. - Cover an unstaged unrelated deletion when a hook runs `git add -A`. --- src/infra/git/repo.ts | 41 ++++++++++++++----------- test/infra/git/repo.integration.test.ts | 30 ++++++++++++++++++ 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 971a180..a557791 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -299,26 +299,31 @@ const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future => resolveHooksDir(root).chain((hooks) => execBin(join(hooks, "pre-commit"), [], indexEnv(tmpIndex), root)); -const resetForeignIndexPaths = (root: string, tmpIndex: string, selected: readonly string[]): Future => - execGitChecked(["-C", root, "ls-files", "-z"], "Failed to list index after hook", indexEnv(tmpIndex)).chain((stdout) => { - const keep = new Set(selected); - return resetIndexPaths( - root, - tmpIndex, - splitNulPaths(stdout).filter((path) => !keep.has(path)) - ); - }); +const listAllIndexPaths = (root: string, tmpIndex: string, failMsg: string): Future => + execGitChecked(["-C", root, "ls-files", "-z"], failMsg, indexEnv(tmpIndex)).map(splitNulPaths); + +const foreignIndexPaths = (selected: readonly string[], preHook: readonly string[], postHook: readonly string[]): readonly string[] => { + const keep = new Set(selected); + return [...new Set([...preHook, ...postHook])].filter((path) => !keep.has(path)); +}; + +const resetForeignIndexPaths = (root: string, tmpIndex: string, selected: readonly string[], preHook: readonly string[]): Future => + listAllIndexPaths(root, tmpIndex, "Failed to list index after hook").chain((postHook) => + resetIndexPaths(root, tmpIndex, foreignIndexPaths(selected, preHook, postHook)) + ); + +const commitIsolatedAfterHook = (root: string, messageFile: string, tmpIndex: string): Future => + Future.bracket(acquireHookIsolation(root), releaseHookIsolation, (iso) => + execBin("git", ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], indexEnv(tmpIndex)) + ); const commitAfterHook = (root: string, messageFile: string, tmpIndex: string, selected: readonly string[]): Future => - runUserPreCommit(root, tmpIndex).chain((hookResult) => - hookResult.either( - () => Future.resolve(hookResult), - () => - resetForeignIndexPaths(root, tmpIndex, selected).chain(() => - Future.bracket(acquireHookIsolation(root), releaseHookIsolation, (iso) => - execBin("git", ["-C", root, "-c", `core.hooksPath=${iso.hooksDir}`, "commit", "-F", messageFile], indexEnv(tmpIndex)) - ) - ) + listAllIndexPaths(root, tmpIndex, "Failed to list index before hook").chain((preHook) => + runUserPreCommit(root, tmpIndex).chain((hookResult) => + hookResult.either( + () => Future.resolve(hookResult), + () => resetForeignIndexPaths(root, tmpIndex, selected, preHook).chain(() => commitIsolatedAfterHook(root, messageFile, tmpIndex)) + ) ) ); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 8b34cef..a7aa386 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -315,6 +315,36 @@ git add -A } }); + it("performCommit with pathspecs ignores hook git add of unstaged unrelated deletions", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "gone.txt"), "tracked\n"); + run("add gone.txt"); + run('commit -m "track gone"'); + writeFileSync(join(dir, "file.txt"), "hello staged\n"); + run("add file.txt"); + unlinkSync(join(dir, "gone.txt")); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: keep", ["file.txt"])); + expect(run("show HEAD:file.txt")).toBe("hello staged\n"); + expect(run("show HEAD:gone.txt")).toBe("tracked\n"); + expect(run("ls-files").trim().split("\n").sort()).toEqual(["file.txt", "gone.txt"]); + expect(run("diff --staged --name-only").trim()).toBe(""); + expect(run("status --porcelain")).toContain("gone.txt"); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs ignores untracked files added by hook git add -A", async () => { const { dir, run } = createTempGitRepo({ staged: false }); writeFileSync(join(dir, "selected.txt"), "keep\n"); From 58c247ebd60e473d9ca483bae5c1585f50a65190 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:02:04 -0300 Subject: [PATCH 30/41] Keep later descendants staged after ancestor file-to-directory commits - Reconcile exact HEAD blobs and force-remove only the named path when HEAD has a tree or no entry. - Cover a first group of `thing` and `thing/a` that must leave `thing/b` staged for the next commit. --- src/infra/git/repo.ts | 18 ++++++++++++++++- test/infra/git/repo.integration.test.ts | 26 +++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index a557791..cc0efcb 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -368,8 +368,24 @@ const resetIndexPaths = (root: string, indexFile: string, paths: readonly string ) ); +const isExactHeadBlob = (lsTree: string): boolean => { + const kind = lsTree.trim().split(/\s+/)[1]; + return kind === "blob" || kind === "commit"; +}; + +const resetExactIndexPath = (root: string, path: string): Future => + execGitChecked(["-C", root, "reset", "-q", "HEAD", "--", path], "Failed to reconcile index after commit").map(() => {}); + +const removeExactIndexPath = (root: string, path: string): Future => + execGitChecked(["-C", root, "update-index", "--force-remove", "--", path], "Failed to reconcile index after commit").map(() => {}); + +const reconcileOneIndexPath = (root: string, path: string): Future => + execGitChecked(["-C", root, "ls-tree", "HEAD", "--", path], "Failed to reconcile index after commit").chain((stdout) => + isExactHeadBlob(stdout) ? resetExactIndexPath(root, path) : removeExactIndexPath(root, path) + ); + const reconcileCommittedIndex = (root: string, paths: readonly string[]): Future => - execGitChecked(["-C", root, "reset", "-q", "HEAD", "--", ...paths], "Failed to reconcile index after commit").map(() => {}); + Future.traverse((path) => reconcileOneIndexPath(root, path), [...paths]).map(() => {}); const finishIsolatedCommit = (root: string, paths: readonly string[], result: ExecResult): Future => result.either( diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index a7aa386..1674426 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -446,6 +446,32 @@ git add -A } }); + it("performCommit with pathspecs keeps later descendants staged after an ancestor file-to-directory commit", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "thing"), "file\n"); + run("add thing"); + run('commit -m "add thing"'); + unlinkSync(join(dir, "thing")); + mkdirSync(join(dir, "thing")); + writeFileSync(join(dir, "thing", "a"), "a\n"); + writeFileSync(join(dir, "thing", "b"), "b\n"); + run("add -A"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: replace file", ["thing", "thing/a"])); + expect(run("show HEAD:thing/a")).toBe("a\n"); + expect(run("cat-file -t HEAD:thing").trim()).toBe("tree"); + expect(() => run("show HEAD:thing/b")).toThrow(); + expect(run("diff --staged --name-only").trim()).toBe("thing/b"); + await runFuture(repo.performCommit("feat: add b", ["thing/b"])); + expect(run("show HEAD:thing/b")).toBe("b\n"); + expect(run("diff --staged --name-only").trim()).toBe(""); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs isolates a directory-to-file replacement when a hook exists", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "thing")); From 67fcd752fac4c7a8d64fa9d745a038d0b145df59 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:08:00 -0300 Subject: [PATCH 31/41] Reset exact index entries and keep case-only rename targets - Classify HEAD paths and force-remove only the named entries so resetting an ancestor cannot restore selected children. - Skip hiding a deleted selected path when it case-aliases a selected index entry on ignorecase filesystems. - Cover a directory-to-file split and a case-only rename with `git add -A`. --- src/infra/git/repo.ts | 163 ++++++++++++++++-------- test/infra/git/repo.integration.test.ts | 50 ++++++++ 2 files changed, 163 insertions(+), 50 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index cc0efcb..1f2559c 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -30,6 +30,7 @@ import { type Result, Failure } from "@/libs/result"; import { absurd } from "@/libs/types"; import { type BaseLookupError } from "@/infra/git/parsers"; import { execBin, type ExecResult } from "@/infra/shell"; +import { spawn } from "node:child_process"; import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; import { access, copyFile, cp, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; @@ -173,16 +174,32 @@ const dirtyPathSet = (root: string, tmpIndex: string, paths: readonly string[]): const hasIndexedDescendant = (path: string, inIndex: ReadonlySet): boolean => [...inIndex].some((indexed) => indexed.startsWith(`${path}/`)); +const hasIndexedCaseAlias = (path: string, inIndex: ReadonlySet): boolean => { + const folded = path.toLowerCase(); + return [...inIndex].some((indexed) => indexed.toLowerCase() === folded); +}; + const planWorktreeHide = ( selected: readonly string[], inIndex: ReadonlySet, - dirty: ReadonlySet + dirty: ReadonlySet, + ignoreCase: boolean ): { checkout: readonly string[]; hide: readonly string[] } => { const checkout = selected.filter((path) => inIndex.has(path) && dirty.has(path)); - const deleted = selected.filter((path) => !inIndex.has(path) && !hasIndexedDescendant(path, inIndex)); + const deleted = selected.filter( + (path) => !inIndex.has(path) && !hasIndexedDescendant(path, inIndex) && !(ignoreCase && hasIndexedCaseAlias(path, inIndex)) + ); return { checkout, hide: [...checkout, ...deleted] }; }; +const repoIgnoresCase = (root: string): Future => + execBin("git", ["-C", root, "config", "--bool", "core.ignorecase"]).map((result) => + result.either( + () => false, + ({ stdout }) => stdout.trim() === "true" + ) + ); + const errnoCode = (err: unknown): string | undefined => (err instanceof Error && "code" in err ? (err as NodeJS.ErrnoException).code : undefined); const isStructuralPathErr = (err: unknown): boolean => { @@ -251,26 +268,28 @@ const backupWorktreeFiles = (root: string, dir: string, paths: readonly string[] const acquireWorktreeSnapshot = (root: string, tmpIndex: string, selected: readonly string[]): Future => { const dir = join(tmpdir(), `commit-wt-${Date.now()}`); - return Future.both(indexPathSet(root, tmpIndex, selected), dirtyPathSet(root, tmpIndex, selected)).chain(([inIndex, dirty]) => { - const { checkout, hide } = planWorktreeHide(selected, inIndex, dirty); - const snap: WorktreeSnapshot = { dir, paths: hide, checkout }; - if (hide.length === 0) { - return Future.resolve(snap); + return Future.both(Future.both(indexPathSet(root, tmpIndex, selected), dirtyPathSet(root, tmpIndex, selected)), repoIgnoresCase(root)).chain( + ([[inIndex, dirty], ignoreCase]) => { + const { checkout, hide } = planWorktreeHide(selected, inIndex, dirty, ignoreCase); + const snap: WorktreeSnapshot = { dir, paths: hide, checkout }; + if (hide.length === 0) { + return Future.resolve(snap); + } + return Future.attemptP(async () => { + await mkdir(dir); + await backupWorktreeFiles(root, dir, hide); + await Promise.all(hide.filter((path) => !checkout.includes(path)).map((rel) => removeWorktreePath(join(root, rel)))); + }) + .chain(() => + checkout.length === 0 ? + Future.resolve(snap) + : execGitChecked(["-C", root, "checkout-index", "-f", "--", ...checkout], "Failed to isolate worktree for hooks", indexEnv(tmpIndex)).map( + () => snap + ) + ) + .chainRej((err) => releaseWorktreeSnapshot(root, snap).chain(() => Future.reject(err))); } - return Future.attemptP(async () => { - await mkdir(dir); - await backupWorktreeFiles(root, dir, hide); - await Promise.all(hide.filter((path) => !checkout.includes(path)).map((rel) => removeWorktreePath(join(root, rel)))); - }) - .chain(() => - checkout.length === 0 ? - Future.resolve(snap) - : execGitChecked(["-C", root, "checkout-index", "-f", "--", ...checkout], "Failed to isolate worktree for hooks", indexEnv(tmpIndex)).map( - () => snap - ) - ) - .chainRej((err) => releaseWorktreeSnapshot(root, snap).chain(() => Future.reject(err))); - }); + ); }; const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future => @@ -349,43 +368,87 @@ const writeNulPathspecFile = (paths: readonly string[]): Future = return file; }); +const execGitStdin = (args: string[], stdin: string, fallbackMsg: string, env?: NodeJS.ProcessEnv): Future => + Future.create((reject, resolve) => { + const proc = spawn("git", args, { + stdio: ["pipe", "pipe", "pipe"], + env: { ...process.env, GIT_LITERAL_PATHSPECS: "1", ...env } + }); + let stdout = ""; + let stderr = ""; + proc.stdout.on("data", (d: Buffer) => (stdout += d.toString())); + proc.stderr.on("data", (d: Buffer) => (stderr += d.toString())); + proc.on("error", (err) => reject(new Error(`Failed to start process: ${err.message}`))); + proc.on("close", (exitCode) => { + if (exitCode === 0) { + resolve(stdout); + return; + } + reject(new Error(stderr.trim() || stdout.trim() || `Command failed with exit code ${exitCode}: ${fallbackMsg}`)); + }); + proc.stdin.end(stdin); + return () => proc.kill(); + }); + +const resetExactHeadBlobs = (root: string, indexFile: string, paths: readonly string[]): Future => + paths.length === 0 ? + Future.resolve(undefined) + : Future.bracket( + writeNulPathspecFile(paths), + (file) => Future.attemptP(() => unlink(file).catch(() => {})), + (file) => + execGitChecked( + ["-C", root, "reset", "-q", "HEAD", `--pathspec-from-file=${file}`, "--pathspec-file-nul"], + "Failed to isolate staged paths", + indexEnv(indexFile) + ).map(() => {}) + ); + +const removeExactIndexPaths = (root: string, indexFile: string, paths: readonly string[]): Future => + paths.length === 0 ? + Future.resolve(undefined) + : execGitStdin( + ["-C", root, "update-index", "--force-remove", "-z", "--stdin"], + `${paths.join("\0")}\0`, + "Failed to isolate staged paths", + indexEnv(indexFile) + ).map(() => {}); + +const partitionHeadIndexPaths = (root: string, paths: readonly string[]): Future => + execGitStdin(["-C", root, "cat-file", "--batch-check"], `${paths.map((path) => `HEAD:${path}`).join("\n")}\n`, "Failed to isolate staged paths").map( + (stdout) => { + const lines = stdout.split("\n"); + const reset: string[] = []; + const remove: string[] = []; + paths.forEach((path, i) => { + const kind = (lines[i] ?? "").split(" ")[1]; + if (kind === "blob" || kind === "commit") { + reset.push(path); + } else { + remove.push(path); + } + }); + return { reset, remove }; + } + ); + const resetIndexPaths = (root: string, indexFile: string, paths: readonly string[]): Future => paths.length === 0 ? Future.resolve(undefined) : execBin("git", ["-C", root, "rev-parse", "-q", "--verify", "HEAD"]).chain((head) => - Future.bracket( - writeNulPathspecFile(paths), - (file) => Future.attemptP(() => unlink(file).catch(() => {})), - (file) => - execGitChecked( - head.either( - () => ["-C", root, "rm", "--cached", "-q", "-f", `--pathspec-from-file=${file}`, "--pathspec-file-nul"], - () => ["-C", root, "reset", "-q", "HEAD", `--pathspec-from-file=${file}`, "--pathspec-file-nul"] - ), - "Failed to isolate staged paths", - indexEnv(indexFile) - ).map(() => {}) + head.either( + () => removeExactIndexPaths(root, indexFile, paths), + () => + partitionHeadIndexPaths(root, paths).chain(({ reset, remove }) => + resetExactHeadBlobs(root, indexFile, reset).chain(() => removeExactIndexPaths(root, indexFile, remove)) + ) ) ); -const isExactHeadBlob = (lsTree: string): boolean => { - const kind = lsTree.trim().split(/\s+/)[1]; - return kind === "blob" || kind === "commit"; -}; - -const resetExactIndexPath = (root: string, path: string): Future => - execGitChecked(["-C", root, "reset", "-q", "HEAD", "--", path], "Failed to reconcile index after commit").map(() => {}); - -const removeExactIndexPath = (root: string, path: string): Future => - execGitChecked(["-C", root, "update-index", "--force-remove", "--", path], "Failed to reconcile index after commit").map(() => {}); - -const reconcileOneIndexPath = (root: string, path: string): Future => - execGitChecked(["-C", root, "ls-tree", "HEAD", "--", path], "Failed to reconcile index after commit").chain((stdout) => - isExactHeadBlob(stdout) ? resetExactIndexPath(root, path) : removeExactIndexPath(root, path) - ); - const reconcileCommittedIndex = (root: string, paths: readonly string[]): Future => - Future.traverse((path) => reconcileOneIndexPath(root, path), [...paths]).map(() => {}); + execGitChecked(["-C", root, "rev-parse", "--absolute-git-dir"], "Failed to reconcile index after commit").chain((gitDir) => + resetIndexPaths(root, join(gitDir.trim(), "index"), paths) + ); const finishIsolatedCommit = (root: string, paths: readonly string[], result: ExecResult): Future => result.either( diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 1674426..4cf31d4 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -170,6 +170,33 @@ git add -A } }); + it("performCommit with pathspecs keeps a case-only rename when a hook runs git add -A", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "other.txt"), "other\n"); + run("add other.txt"); + run("mv -f file.txt File.txt"); + const hookPath = join(dir, ".git", "hooks", "pre-commit"); + writeFileSync( + hookPath, + `#!/bin/sh +git add -A +` + ); + chmodSync(hookPath, 0o755); + const prev = cwd(); + chdir(dir); + try { + const staged = await runFuture(repo.listStagedPaths()); + expect([...staged].sort()).toEqual(["File.txt", "file.txt", "other.txt"]); + await runFuture(repo.performCommit("feat: case rename", ["file.txt", "File.txt"])); + expect(run("ls-tree -r --name-only HEAD").trim()).toBe("File.txt"); + expect(run("show HEAD:File.txt")).toBe("hello\n"); + expect(run("diff --staged --name-only").trim()).toBe("other.txt"); + } finally { + chdir(prev); + } + }); + it("performCommit with paths records the staged blob not the worktree", async () => { const { dir, run } = createTempGitRepo({ staged: true }); writeFileSync(join(dir, "file.txt"), "hello unstaged secret\n"); @@ -472,6 +499,29 @@ git add -A } }); + it("performCommit with pathspecs keeps a later ancestor file after committing a directory deletion", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + mkdirSync(join(dir, "thing")); + writeFileSync(join(dir, "thing", "a"), "a\n"); + run("add thing"); + run('commit -m "add dir"'); + run("rm -rf thing"); + writeFileSync(join(dir, "thing"), "file\n"); + run("add -A"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: delete child", ["thing/a"])); + expect(() => run("show HEAD:thing/a")).toThrow(); + expect(run("diff --staged --name-only").trim()).toBe("thing"); + await runFuture(repo.performCommit("feat: add file", ["thing"])); + expect(run("show HEAD:thing")).toBe("file\n"); + expect(run("diff --staged --name-only").trim()).toBe(""); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs isolates a directory-to-file replacement when a hook exists", async () => { const { dir, run } = createTempGitRepo({ staged: false }); mkdirSync(join(dir, "thing")); From 0ae8c887f920921521e434c1bad1fec71071d5b4 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:20:39 -0300 Subject: [PATCH 32/41] Restore HEAD blobs as exact index entries during isolate - Replace recursive `git reset` pathspecs with `update-index --index-info` from `ls-tree`. - Skip restoring an ancestor blob when a selected path is a descendant so `thing/a` can commit first. - Cover a file-to-directory split that isolates `thing/a` and leaves `thing/b` staged. --- src/infra/git/repo.ts | 94 +++++++++++++------------ test/infra/git/repo.integration.test.ts | 25 +++++++ 2 files changed, 75 insertions(+), 44 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 1f2559c..133caf6 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -328,7 +328,7 @@ const foreignIndexPaths = (selected: readonly string[], preHook: readonly string const resetForeignIndexPaths = (root: string, tmpIndex: string, selected: readonly string[], preHook: readonly string[]): Future => listAllIndexPaths(root, tmpIndex, "Failed to list index after hook").chain((postHook) => - resetIndexPaths(root, tmpIndex, foreignIndexPaths(selected, preHook, postHook)) + resetIndexPaths(root, tmpIndex, foreignIndexPaths(selected, preHook, postHook), new Set(selected)) ); const commitIsolatedAfterHook = (root: string, messageFile: string, tmpIndex: string): Future => @@ -361,13 +361,6 @@ const copyIndexFile = (root: string, dest: string): Future => const listStagedPathsNoRenames = (root: string): Future => execGitChecked(["-C", root, "diff", "--staged", "--name-only", "--no-renames", "-z"], "Failed to list staged files").map(splitNulPaths); -const writeNulPathspecFile = (paths: readonly string[]): Future => - Future.attemptP(async () => { - const file = join(tmpdir(), `commit-pathspec-${Date.now()}`); - await writeFile(file, `${paths.join("\0")}\0`); - return file; - }); - const execGitStdin = (args: string[], stdin: string, fallbackMsg: string, env?: NodeJS.ProcessEnv): Future => Future.create((reject, resolve) => { const proc = spawn("git", args, { @@ -390,19 +383,38 @@ const execGitStdin = (args: string[], stdin: string, fallbackMsg: string, env?: return () => proc.kill(); }); -const resetExactHeadBlobs = (root: string, indexFile: string, paths: readonly string[]): Future => - paths.length === 0 ? +type HeadBlob = { mode: string; sha: string }; + +const parseHeadBlobMap = (stdout: string): ReadonlyMap => { + const map = new Map(); + for (const rec of splitNulPaths(stdout)) { + const tab = rec.indexOf("\t"); + if (tab < 0) { + continue; + } + const [mode, kind, sha] = rec.slice(0, tab).split(" "); + const path = rec.slice(tab + 1); + if ((kind === "blob" || kind === "commit") && mode && sha && path) { + map.set(path, { mode, sha }); + } + } + return map; +}; + +const listHeadBlobMap = (root: string): Future> => + execGitChecked(["-C", root, "ls-tree", "-r", "-z", "HEAD"], "Failed to isolate staged paths").map(parseHeadBlobMap); + +const hasKeptDescendant = (path: string, keep: ReadonlySet): boolean => [...keep].some((kept) => kept.startsWith(`${path}/`)); + +const restoreExactHeadBlobs = (root: string, indexFile: string, entries: readonly (HeadBlob & { path: string })[]): Future => + entries.length === 0 ? Future.resolve(undefined) - : Future.bracket( - writeNulPathspecFile(paths), - (file) => Future.attemptP(() => unlink(file).catch(() => {})), - (file) => - execGitChecked( - ["-C", root, "reset", "-q", "HEAD", `--pathspec-from-file=${file}`, "--pathspec-file-nul"], - "Failed to isolate staged paths", - indexEnv(indexFile) - ).map(() => {}) - ); + : execGitStdin( + ["-C", root, "update-index", "-z", "--index-info"], + `${entries.map((entry) => `${entry.mode} ${entry.sha}\t${entry.path}`).join("\0")}\0`, + "Failed to isolate staged paths", + indexEnv(indexFile) + ).map(() => {}); const removeExactIndexPaths = (root: string, indexFile: string, paths: readonly string[]): Future => paths.length === 0 ? @@ -414,40 +426,34 @@ const removeExactIndexPaths = (root: string, indexFile: string, paths: readonly indexEnv(indexFile) ).map(() => {}); -const partitionHeadIndexPaths = (root: string, paths: readonly string[]): Future => - execGitStdin(["-C", root, "cat-file", "--batch-check"], `${paths.map((path) => `HEAD:${path}`).join("\n")}\n`, "Failed to isolate staged paths").map( - (stdout) => { - const lines = stdout.split("\n"); - const reset: string[] = []; - const remove: string[] = []; - paths.forEach((path, i) => { - const kind = (lines[i] ?? "").split(" ")[1]; - if (kind === "blob" || kind === "commit") { - reset.push(path); - } else { - remove.push(path); - } - }); - return { reset, remove }; - } - ); +const applyHeadIndexPartition = ( + root: string, + indexFile: string, + paths: readonly string[], + keep: ReadonlySet, + head: ReadonlyMap +): Future => { + const restore = paths.flatMap((path) => { + const blob = head.get(path); + return blob === undefined || hasKeptDescendant(path, keep) ? [] : [{ path, ...blob }]; + }); + const remove = paths.filter((path) => head.get(path) === undefined); + return removeExactIndexPaths(root, indexFile, remove).chain(() => restoreExactHeadBlobs(root, indexFile, restore)); +}; -const resetIndexPaths = (root: string, indexFile: string, paths: readonly string[]): Future => +const resetIndexPaths = (root: string, indexFile: string, paths: readonly string[], keep: ReadonlySet): Future => paths.length === 0 ? Future.resolve(undefined) : execBin("git", ["-C", root, "rev-parse", "-q", "--verify", "HEAD"]).chain((head) => head.either( () => removeExactIndexPaths(root, indexFile, paths), - () => - partitionHeadIndexPaths(root, paths).chain(({ reset, remove }) => - resetExactHeadBlobs(root, indexFile, reset).chain(() => removeExactIndexPaths(root, indexFile, remove)) - ) + () => listHeadBlobMap(root).chain((blobs) => applyHeadIndexPartition(root, indexFile, paths, keep, blobs)) ) ); const reconcileCommittedIndex = (root: string, paths: readonly string[]): Future => execGitChecked(["-C", root, "rev-parse", "--absolute-git-dir"], "Failed to reconcile index after commit").chain((gitDir) => - resetIndexPaths(root, join(gitDir.trim(), "index"), paths) + resetIndexPaths(root, join(gitDir.trim(), "index"), paths, new Set()) ); const finishIsolatedCommit = (root: string, paths: readonly string[], result: ExecResult): Future => @@ -460,7 +466,7 @@ const isolateAndCommit = (root: string, messageFile: string, tmpIndex: string, p listStagedPathsNoRenames(root).chain((staged) => { const keep = new Set(paths); const unselected = staged.filter((path) => !keep.has(path)); - return resetIndexPaths(root, tmpIndex, unselected).chain(() => + return resetIndexPaths(root, tmpIndex, unselected, keep).chain(() => commitIsolatedIndex(root, messageFile, tmpIndex, paths).chain((result) => finishIsolatedCommit(root, paths, result)) ); }); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index 4cf31d4..dc96e5c 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -473,6 +473,31 @@ git add -A } }); + it("performCommit with pathspecs commits a file-to-directory child when the ancestor is unselected", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "thing"), "file\n"); + run("add thing"); + run('commit -m "add thing"'); + unlinkSync(join(dir, "thing")); + mkdirSync(join(dir, "thing")); + writeFileSync(join(dir, "thing", "a"), "a\n"); + writeFileSync(join(dir, "thing", "b"), "b\n"); + run("add -A"); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: add a", ["thing/a"])); + expect(run("show HEAD:thing/a")).toBe("a\n"); + expect(run("cat-file -t HEAD:thing").trim()).toBe("tree"); + expect(run("diff --staged --name-only").trim()).toBe("thing/b"); + await runFuture(repo.performCommit("feat: add b", ["thing/b"])); + expect(run("show HEAD:thing/b")).toBe("b\n"); + expect(run("diff --staged --name-only").trim()).toBe(""); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs keeps later descendants staged after an ancestor file-to-directory commit", async () => { const { dir, run } = createTempGitRepo({ staged: false }); writeFileSync(join(dir, "thing"), "file\n"); From e87d5ebf8579b26bfd83e888c871db424a46a420 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:35:11 -0300 Subject: [PATCH 33/41] Fix isolated commit empty groups and hook execution - No-op isolate when none of the selected paths remain staged. - Wrap copied hooks so they exec the original path and keep `$0`. - Run the user pre-commit hook through `git hook run`. - Cover a consumed ancestor group and a commit-msg helper next to the real hook. --- src/infra/git/repo.ts | 23 ++++++++++++------ test/infra/git/repo.integration.test.ts | 32 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 133caf6..3e0bd2f 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -26,14 +26,14 @@ export { import { Future } from "@/libs/future"; import { Just, Nothing, type Maybe } from "@/libs/maybe"; -import { type Result, Failure } from "@/libs/result"; +import { type Result, Failure, Success } from "@/libs/result"; import { absurd } from "@/libs/types"; import { type BaseLookupError } from "@/infra/git/parsers"; import { execBin, type ExecResult } from "@/infra/shell"; import { spawn } from "node:child_process"; import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; -import { access, copyFile, cp, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; +import { access, chmod, copyFile, cp, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; import { @@ -123,13 +123,18 @@ const resolveHooksDir = (root: string): Future => return isAbsolute(trimmed) ? trimmed : join(root, trimmed); }); +const shellSingleQuote = (value: string): string => `'${value.replace(/'/g, `'\\''`)}'`; + const copyHookEntry = async (src: string, dest: string): Promise => { const st = await lstat(src); if (st.isDirectory()) { - await cp(src, dest, { recursive: true }); + await mkdir(dest, { recursive: true }); + const names = await readdir(src); + await Promise.all(names.map((name) => copyHookEntry(join(src, name), join(dest, name)))); return; } - await copyFile(src, dest); + await writeFile(dest, `#!/bin/sh\nexec ${shellSingleQuote(src)} "$@"\n`); + await chmod(dest, st.mode); }; const acquireHookIsolation = (root: string): Future => { @@ -316,7 +321,7 @@ const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future => - resolveHooksDir(root).chain((hooks) => execBin(join(hooks, "pre-commit"), [], indexEnv(tmpIndex), root)); + execBin("git", ["-C", root, "hook", "run", "pre-commit"], indexEnv(tmpIndex)); const listAllIndexPaths = (root: string, tmpIndex: string, failMsg: string): Future => execGitChecked(["-C", root, "ls-files", "-z"], failMsg, indexEnv(tmpIndex)).map(splitNulPaths); @@ -464,10 +469,14 @@ const finishIsolatedCommit = (root: string, paths: readonly string[], result: Ex const isolateAndCommit = (root: string, messageFile: string, tmpIndex: string, paths: readonly string[]): Future => listStagedPathsNoRenames(root).chain((staged) => { - const keep = new Set(paths); + const stagedSet = new Set(staged); + const keep = new Set(paths.filter((path) => stagedSet.has(path))); + if (keep.size === 0) { + return Future.resolve(Success({ stdout: "", stderr: "" })); + } const unselected = staged.filter((path) => !keep.has(path)); return resetIndexPaths(root, tmpIndex, unselected, keep).chain(() => - commitIsolatedIndex(root, messageFile, tmpIndex, paths).chain((result) => finishIsolatedCommit(root, paths, result)) + commitIsolatedIndex(root, messageFile, tmpIndex, [...keep]).chain((result) => finishIsolatedCommit(root, [...keep], result)) ); }); diff --git a/test/infra/git/repo.integration.test.ts b/test/infra/git/repo.integration.test.ts index dc96e5c..4c6425f 100644 --- a/test/infra/git/repo.integration.test.ts +++ b/test/infra/git/repo.integration.test.ts @@ -420,6 +420,36 @@ test -n "$files" } }); + it("performCommit with pathspecs runs commit-msg from its original hook path", async () => { + const { dir, run } = createTempGitRepo({ staged: false }); + writeFileSync(join(dir, "a.txt"), "a\n"); + writeFileSync(join(dir, "b.txt"), "b\n"); + run("add a.txt b.txt"); + mkdirSync(join(dir, ".git", "hook-tools")); + writeFileSync(join(dir, ".git", "hook-tools", "check-msg"), "#!/bin/sh\nexit 0\n"); + chmodSync(join(dir, ".git", "hook-tools", "check-msg"), 0o755); + writeFileSync(join(dir, ".git", "hooks", "pre-commit"), "#!/bin/sh\nexit 0\n"); + chmodSync(join(dir, ".git", "hooks", "pre-commit"), 0o755); + writeFileSync( + join(dir, ".git", "hooks", "commit-msg"), + `#!/bin/sh +helper=$(dirname "$0")/../hook-tools/check-msg +test -x "$helper" && exec "$helper" +exit 1 +` + ); + chmodSync(join(dir, ".git", "hooks", "commit-msg"), 0o755); + const prev = cwd(); + chdir(dir); + try { + await runFuture(repo.performCommit("feat: original hook path", ["a.txt"])); + expect(run("show HEAD:a.txt")).toBe("a\n"); + expect(run("diff --staged --name-only").trim()).toBe("b.txt"); + } finally { + chdir(prev); + } + }); + it("performCommit with pathspecs isolates a file-to-directory replacement when a hook exists", async () => { const { dir, run } = createTempGitRepo({ staged: false }); writeFileSync(join(dir, "thing"), "file\n"); @@ -490,6 +520,8 @@ git add -A expect(run("show HEAD:thing/a")).toBe("a\n"); expect(run("cat-file -t HEAD:thing").trim()).toBe("tree"); expect(run("diff --staged --name-only").trim()).toBe("thing/b"); + await runFuture(repo.performCommit("feat: delete thing", ["thing"])); + expect(run("diff --staged --name-only").trim()).toBe("thing/b"); await runFuture(repo.performCommit("feat: add b", ["thing/b"])); expect(run("show HEAD:thing/b")).toBe("b\n"); expect(run("diff --staged --name-only").trim()).toBe(""); From 55f082b5c8b337debe2159861145b753afc59a2b Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:43:43 -0300 Subject: [PATCH 34/41] Fall back when git hook run is unavailable - Run the hook file directly if `git hook run` is not a git command. --- src/infra/git/repo.ts | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 3e0bd2f..42d526b 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -29,7 +29,7 @@ import { Just, Nothing, type Maybe } from "@/libs/maybe"; import { type Result, Failure, Success } from "@/libs/result"; import { absurd } from "@/libs/types"; import { type BaseLookupError } from "@/infra/git/parsers"; -import { execBin, type ExecResult } from "@/infra/shell"; +import { execBin, type CommandFailure, type ExecResult } from "@/infra/shell"; import { spawn } from "node:child_process"; import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; @@ -320,8 +320,21 @@ const releaseWorktreeSnapshot = (root: string, snap: WorktreeSnapshot): Future { + const text = `${failure.output.stderr}\n${failure.output.stdout}`.toLowerCase(); + return text.includes("is not a git command") && text.includes("hook"); +}; + +const runPreCommitFile = (root: string, tmpIndex: string): Future => + resolveHooksDir(root).chain((hooks) => execBin(join(hooks, "pre-commit"), [], indexEnv(tmpIndex), root)); + const runUserPreCommit = (root: string, tmpIndex: string): Future => - execBin("git", ["-C", root, "hook", "run", "pre-commit"], indexEnv(tmpIndex)); + execBin("git", ["-C", root, "hook", "run", "pre-commit"], indexEnv(tmpIndex)).chain((result) => + result.either( + (failure) => (isMissingGitHookCommand(failure) ? runPreCommitFile(root, tmpIndex) : Future.resolve(result)), + (ok) => Future.resolve(Success(ok)) + ) + ); const listAllIndexPaths = (root: string, tmpIndex: string, failMsg: string): Future => execGitChecked(["-C", root, "ls-files", "-z"], failMsg, indexEnv(tmpIndex)).map(splitNulPaths); From cbea34d52e8a2f5698fc52a0e25121259fdd7d48 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:53:40 -0300 Subject: [PATCH 35/41] Run the old-Git pre-commit fallback through sh - Invoke `sh` on the hook file so a `#!/bin/sh` shebang still runs without `git hook run`. --- src/infra/git/repo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 42d526b..8034420 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -326,7 +326,7 @@ const isMissingGitHookCommand = (failure: CommandFailure): boolean => { }; const runPreCommitFile = (root: string, tmpIndex: string): Future => - resolveHooksDir(root).chain((hooks) => execBin(join(hooks, "pre-commit"), [], indexEnv(tmpIndex), root)); + resolveHooksDir(root).chain((hooks) => execBin("sh", [join(hooks, "pre-commit")], indexEnv(tmpIndex), root)); const runUserPreCommit = (root: string, tmpIndex: string): Future => execBin("git", ["-C", root, "hook", "run", "pre-commit"], indexEnv(tmpIndex)).chain((result) => From 306a2c077bba2140fe487268febe48828e476133 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Wed, 12 Aug 2026 23:58:35 -0300 Subject: [PATCH 36/41] Honor hook shebangs on the old-Git fallback - Exec the pre-commit file directly on Unix so Python and Node hooks still run. - Keep the `sh` fallback on Windows where spawn cannot launch extensionless hooks. --- src/infra/git/repo.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 8034420..63f99ca 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -326,7 +326,10 @@ const isMissingGitHookCommand = (failure: CommandFailure): boolean => { }; const runPreCommitFile = (root: string, tmpIndex: string): Future => - resolveHooksDir(root).chain((hooks) => execBin("sh", [join(hooks, "pre-commit")], indexEnv(tmpIndex), root)); + resolveHooksDir(root).chain((hooks) => { + const hook = join(hooks, "pre-commit"); + return process.platform === "win32" ? execBin("sh", [hook], indexEnv(tmpIndex), root) : execBin(hook, [], indexEnv(tmpIndex), root); + }); const runUserPreCommit = (root: string, tmpIndex: string): Future => execBin("git", ["-C", root, "hook", "run", "pre-commit"], indexEnv(tmpIndex)).chain((result) => From 049cc4f68c3d6d31feba1312bb1a51cba2a69d53 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 13 Aug 2026 00:10:16 -0300 Subject: [PATCH 37/41] Dispatch Windows fallback hooks from their shebang - Parse the hook shebang and invoke that interpreter on the old-Git Windows fallback. - Keep `sh` for missing or env-only shebangs. - Cover interpreter selection with unit tests. --- src/infra/git/parsers.ts | 11 +++++++++++ src/infra/git/repo.ts | 13 ++++++++++--- test/infra/git/parsers.test.ts | 21 ++++++++++++++++++++- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/infra/git/parsers.ts b/src/infra/git/parsers.ts index 2276a70..e6a11a3 100644 --- a/src/infra/git/parsers.ts +++ b/src/infra/git/parsers.ts @@ -8,6 +8,7 @@ export { parseRemoteFromUpstream, splitCommitFields, commandFailureMessage, + parseHookInterpreter, CREATED_FROM_RE, COMMIT_KEYS, type BaseLookupError @@ -28,6 +29,16 @@ const COMMIT_KEYS = ["hash", "short", "subject", "authorName", "authorEmail", "d const commandFailureMessage = (failure: CommandFailure, fallbackMsg: string): string => failure.output.stderr.trim() || failure.output.stdout.trim() || `${failure.error.message}: ${fallbackMsg}`; +const lastPathSegment = (path: string): string => path.split(/[/\\]/).filter(Boolean).at(-1) ?? path; + +const parseHookInterpreter = (shebangLine: string): string => { + const line = shebangLine.trim(); + const env = /^#!\s*\/usr\/bin\/env(?:\s+(\S+))?/.exec(line); + if (env) return env[1] === undefined || env[1] === "" ? "sh" : lastPathSegment(env[1]); + const interp = /^#!\s*(\S+)/.exec(line); + return interp?.[1] === undefined ? "sh" : lastPathSegment(interp[1]); +}; + const formatCommitOutput = (stdout: string): string => "\n" + stdout diff --git a/src/infra/git/repo.ts b/src/infra/git/repo.ts index 63f99ca..655394f 100644 --- a/src/infra/git/repo.ts +++ b/src/infra/git/repo.ts @@ -33,7 +33,7 @@ import { execBin, type CommandFailure, type ExecResult } from "@/infra/shell"; import { spawn } from "node:child_process"; import * as Decoder from "@/libs/json/decoder"; import { constants as fsConstants } from "node:fs"; -import { access, chmod, copyFile, cp, lstat, mkdir, readdir, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; +import { access, chmod, copyFile, cp, lstat, mkdir, readdir, readFile, readlink, rm, symlink, unlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, isAbsolute, join } from "node:path"; import { @@ -42,7 +42,8 @@ import { parseBaseFromReflog, parseRemoteFromUpstream, splitCommitFields, - commandFailureMessage + commandFailureMessage, + parseHookInterpreter } from "@/infra/git/parsers"; type CommitMetadata = { @@ -325,10 +326,16 @@ const isMissingGitHookCommand = (failure: CommandFailure): boolean => { return text.includes("is not a git command") && text.includes("hook"); }; +const runWindowsPreCommitFile = (hook: string, env: NodeJS.ProcessEnv, root: string): Future => + Future.attemptP(() => readFile(hook, "utf8")) + .chainRej(() => Future.resolve("")) + .chain((text) => execBin(parseHookInterpreter(text.split(/\r?\n/, 1)[0] ?? ""), [hook], env, root)); + const runPreCommitFile = (root: string, tmpIndex: string): Future => resolveHooksDir(root).chain((hooks) => { const hook = join(hooks, "pre-commit"); - return process.platform === "win32" ? execBin("sh", [hook], indexEnv(tmpIndex), root) : execBin(hook, [], indexEnv(tmpIndex), root); + const env = indexEnv(tmpIndex); + return process.platform === "win32" ? runWindowsPreCommitFile(hook, env, root) : execBin(hook, [], env, root); }); const runUserPreCommit = (root: string, tmpIndex: string): Future => diff --git a/test/infra/git/parsers.test.ts b/test/infra/git/parsers.test.ts index c56a172..b47a436 100644 --- a/test/infra/git/parsers.test.ts +++ b/test/infra/git/parsers.test.ts @@ -5,7 +5,8 @@ import { parseBaseFromReflog, splitCommitFields, parseRemoteFromUpstream, - commandFailureMessage + commandFailureMessage, + parseHookInterpreter } from "@/infra/git/parsers"; import { Just, Nothing } from "@/libs/maybe"; import { Success } from "@/libs/result"; @@ -66,3 +67,21 @@ describe("commandFailureMessage", () => { expect(msg).toBe("fatal: no repo"); }); }); + +describe("parseHookInterpreter", () => { + it("defaults to sh without a shebang", () => { + expect(parseHookInterpreter("exit 0")).toBe("sh"); + }); + + it("uses env program and path basename", () => { + expect(parseHookInterpreter("#!/usr/bin/env python3")).toBe("python3"); + expect(parseHookInterpreter("#! /usr/bin/env node")).toBe("node"); + expect(parseHookInterpreter("#!/usr/bin/env")).toBe("sh"); + }); + + it("uses the interpreter basename for direct shebangs", () => { + expect(parseHookInterpreter("#!/bin/sh")).toBe("sh"); + expect(parseHookInterpreter("#!/bin/bash")).toBe("bash"); + expect(parseHookInterpreter("#!/usr/bin/python")).toBe("python"); + }); +}); From ec8d576b7cb13b44d569730e13933385f57aa499 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 13 Aug 2026 08:36:58 -0300 Subject: [PATCH 38/41] Extract routeAnalysis for split vs single commit plans - Move split-vs-single routing into `routeAnalysis` returning `AnalysisRoute`. - Fail empty plans with a `Result` instead of an inline `Future.reject`. - Cover split, single, shouldSplit-with-one-commit, and empty-plan cases. --- src/cli/commit.ts | 38 ++++++++++++++++++++++-------- test/cli/commit.test.ts | 51 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/cli/commit.ts b/src/cli/commit.ts index 7b589c7..08d6077 100644 --- a/src/cli/commit.ts +++ b/src/cli/commit.ts @@ -1,4 +1,4 @@ -export { Commit }; +export { Commit, routeAnalysis, type AnalysisRoute }; import * as p from "@clack/prompts"; import * as pr from "@/infra/github/pr"; @@ -18,7 +18,10 @@ import { type LlmRequestMetadata, type SplitPlanContent } from "@/domain/llm/router"; -import { Nothing, type Maybe, Just } from "@/libs/maybe"; +import { type SplitPlan } from "@/domain/split/plan"; +import { Nothing, type Maybe, Just, fromOptional } from "@/libs/maybe"; +import { Failure, Success, type Result } from "@/libs/result"; +import { absurd } from "@/libs/types"; import { loading } from "@/infra/ui/spinner"; import { renderCommitNote, renderPushNote } from "@/infra/ui/push-note"; @@ -27,6 +30,16 @@ import color from "picocolors"; const USER_ACTIONS = ["commit_push", "commit", "split", "regenerate", "adjust", "cancel"] as const; type UserAction = (typeof USER_ACTIONS)[number]; +type AnalysisRoute = { tag: "split"; plan: SplitPlan } | { tag: "single"; message: string }; + +const routeAnalysis = (plan: SplitPlan): Result => + plan.shouldSplit && plan.commits.length >= 2 ? + Success({ tag: "split", plan }) + : fromOptional(plan.commits[0]).unwrap>( + () => Failure(new Error("Split plan: expected at least 1 commit")), + (first) => Success({ tag: "single", message: first.message }) + ); + class Commit { private constructor( private readonly config: Config, @@ -67,14 +80,19 @@ class Commit { private followAnalysis(diff: string, files: readonly string[], content: SplitPlanContent): Future { const { plan, metadata } = content; - if (plan.shouldSplit && plan.commits.length >= 2) { - return Split.fromResolved(this.config, this.providerConfig).runPlan(diff, files, plan, metadata); - } - const first = plan.commits[0]; - if (first === undefined) { - return Future.reject(new Error("Split plan: expected at least 1 commit")); - } - return this.interact(diff, files, { text: first.message, metadata }); + return routeAnalysis(plan).either( + (e) => Future.reject(e), + (route) => { + switch (route.tag) { + case "split": + return Split.fromResolved(this.config, this.providerConfig).runPlan(diff, files, route.plan, metadata); + case "single": + return this.interact(diff, files, { text: route.message, metadata }); + default: + return absurd(route, "AnalysisRoute"); + } + } + ); } private startSplit(diff: string, files: readonly string[]): Future { diff --git a/test/cli/commit.test.ts b/test/cli/commit.test.ts index 7c2ca8d..ce46f61 100644 --- a/test/cli/commit.test.ts +++ b/test/cli/commit.test.ts @@ -4,9 +4,10 @@ vi.mock("@/infra/env", () => ({ environment: { GOOGLE_CLIENT_ID: "test", GOOGLE_CLIENT_SECRET: "test" } })); -import { Commit } from "@/cli/commit"; +import { Commit, routeAnalysis } from "@/cli/commit"; import { Future } from "@/libs/future"; import { Nothing, Just } from "@/libs/maybe"; +import { Failure, Success } from "@/libs/result"; import { runFuture } from "@test/helpers/run-future"; import * as s from "@/libs/json/schema"; import { Config } from "@/domain/config/config"; @@ -196,3 +197,51 @@ describe("Commit.run", () => { expect(repo.performCommit).toHaveBeenNthCalledWith(2, "msg two", ["b.ts"]); }); }); + +describe("routeAnalysis", () => { + const twoCommits = [ + { message: "msg one", files: ["a.ts"] }, + { message: "msg two", files: ["b.ts"] } + ] as const; + + it("routes a multi-commit split plan to split", () => { + const plan = { shouldSplit: true, commits: twoCommits }; + const r = routeAnalysis(plan); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value).toEqual({ tag: "split", plan }); + } + }); + + it("routes a single-commit plan to that message", () => { + const r = routeAnalysis({ shouldSplit: false, commits: [{ message: "feat: one", files: ["a.ts"] }] }); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value).toEqual({ tag: "single", message: "feat: one" }); + } + }); + + it("does not split when shouldSplit is true but only one commit exists", () => { + const r = routeAnalysis({ shouldSplit: true, commits: [{ message: "feat: one", files: ["a.ts"] }] }); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value).toEqual({ tag: "single", message: "feat: one" }); + } + }); + + it("uses the first message when shouldSplit is false with multiple commits", () => { + const r = routeAnalysis({ shouldSplit: false, commits: twoCommits }); + expect(r instanceof Success).toBe(true); + if (r instanceof Success) { + expect(r.value).toEqual({ tag: "single", message: "msg one" }); + } + }); + + it("fails when the plan has no commits", () => { + const r = routeAnalysis({ shouldSplit: false, commits: [] }); + expect(r instanceof Failure).toBe(true); + if (r instanceof Failure) { + expect(r.error.message).toBe("Split plan: expected at least 1 commit"); + } + }); +}); From dd2970f9723cdbc234c5374e75c6a7dd8d245392 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 13 Aug 2026 08:48:23 -0300 Subject: [PATCH 39/41] Remove standalone `commit split` command - Drop the `split` CLI command from the parser, entrypoint, and alias targets. - Update README command docs to match. --- README.md | 3 +-- index.ts | 5 +---- src/cli/parser.ts | 4 ---- src/domain/alias/alias.ts | 4 +--- test/cli/parser.test.ts | 1 - 5 files changed, 3 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index a8b78b3..5cd5f0b 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ Or explicitly: commit generate ``` -With split enabled in setup, `commit` analyzes staged files and opens a multi-commit plan when they look independent. `commit split` always opens that plan. From a single-message prompt you can also pick Split. +With split enabled in setup, `commit` analyzes staged files and opens a multi-commit plan when they look independent. ### System Checks @@ -201,7 +201,6 @@ commit --help | ------------------------ | ------------------------------------------------- | | `commit` | Generate a commit message (default) | | `commit generate` | Generate a commit message | -| `commit split` | Force a multi-commit plan from staged changes | | `commit setup` | Configure authentication and conventions | | `commit login` | Alias for setup — re-authenticate | | `commit doctor` | Check installation and environment | diff --git a/index.ts b/index.ts index faec76a..f2bfe8a 100755 --- a/index.ts +++ b/index.ts @@ -1,5 +1,4 @@ import { Commit } from "@/cli/commit"; -import { Split } from "@/cli/split"; import { Branch } from "@/cli/branch"; import { Setup } from "@/cli/setup"; import { Doctor } from "@/cli/doctor"; @@ -14,7 +13,7 @@ import { checkUpdate } from "@/cli/update"; import color from "picocolors"; -const NOTIFIER_COMMANDS = new Set(["generate", "split", "setup", "doctor", "model", "effort", "branch", "alias"]); +const NOTIFIER_COMMANDS = new Set(["generate", "setup", "doctor", "model", "effort", "branch", "alias"]); const main = () => { const args = process.argv.slice(2); @@ -30,8 +29,6 @@ const main = () => { switch (command.type) { case "generate": return Commit.create().chain((c) => c.run()); - case "split": - return Split.create().chain((s) => s.run()); case "setup": return Setup.create().chain((s) => s.run()); case "doctor": diff --git a/src/cli/parser.ts b/src/cli/parser.ts index 3161201..2e80b31 100644 --- a/src/cli/parser.ts +++ b/src/cli/parser.ts @@ -10,7 +10,6 @@ type AliasAction = { type: "hub" } | { type: "list" } | { type: "add"; name: str type CliCommand = | { type: "generate" } - | { type: "split" } | { type: "setup" } | { type: "doctor" } | { type: "model" } @@ -54,8 +53,6 @@ const cliCommandDecoder: D.Decoder = D.array(D.string).chain((args) switch (cmd) { case "generate": return D.succeed({ type: "generate" }); - case "split": - return D.succeed({ type: "split" }); case "setup": case "login": return D.succeed({ type: "setup" }); @@ -92,7 +89,6 @@ Usage: commit-tools [command] Commands: generate (default) Generate a commit message - split Force a multi-commit plan from staged changes branch Suggest branch names from local changes and create one new-branch Alias for branch setup Configure authentication and conventions diff --git a/src/domain/alias/alias.ts b/src/domain/alias/alias.ts index 4f30875..c0fba87 100644 --- a/src/domain/alias/alias.ts +++ b/src/domain/alias/alias.ts @@ -6,7 +6,7 @@ import { Failure, Success, type Result } from "@/libs/result"; import { fromOptional, Just, type Maybe } from "@/libs/maybe"; import { absurd } from "@/libs/types"; -const ALIAS_TARGETS = ["generate", "split", "branch", "setup", "doctor", "model", "effort", "update"] as const; +const ALIAS_TARGETS = ["generate", "branch", "setup", "doctor", "model", "effort", "update"] as const; type AliasTarget = (typeof ALIAS_TARGETS)[number]; const NAME_PATTERN = /^[a-zA-Z][a-zA-Z0-9_-]{0,31}$/; @@ -57,8 +57,6 @@ const describeTarget = (target: AliasTarget): string => { switch (target) { case "generate": return "Generate a commit message"; - case "split": - return "Split staged changes into multiple commits"; case "branch": return "Suggest branch names and create one"; case "setup": diff --git a/test/cli/parser.test.ts b/test/cli/parser.test.ts index 61325c0..b6a4992 100644 --- a/test/cli/parser.test.ts +++ b/test/cli/parser.test.ts @@ -6,7 +6,6 @@ import { Failure, Success } from "@/libs/result"; describe("parseArgs", () => { it.each([ [["generate"], "generate"], - [["split"], "split"], [["branch"], "branch"], [["new-branch"], "branch"], [["setup"], "setup"], From 109ee976ab2ff5686cc11dd5d312d229e067ab00 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 13 Aug 2026 08:48:23 -0300 Subject: [PATCH 40/41] Remove manual Split action from generate flow - Stop offering Split after a single-message prompt and delete `startSplit`. - Keep `Split` as an analysis-only runner via `fromResolved` and `runPlan`. - Update commit and split tests accordingly. --- src/cli/commit.ts | 33 ++++++++++------------------- src/cli/split.ts | 25 ---------------------- test/cli/commit.test.ts | 27 ------------------------ test/cli/split.test.ts | 46 ++++++++++++++++------------------------- 4 files changed, 29 insertions(+), 102 deletions(-) diff --git a/src/cli/commit.ts b/src/cli/commit.ts index 08d6077..8904049 100644 --- a/src/cli/commit.ts +++ b/src/cli/commit.ts @@ -27,7 +27,7 @@ import { renderCommitNote, renderPushNote } from "@/infra/ui/push-note"; import color from "picocolors"; -const USER_ACTIONS = ["commit_push", "commit", "split", "regenerate", "adjust", "cancel"] as const; +const USER_ACTIONS = ["commit_push", "commit", "regenerate", "adjust", "cancel"] as const; type UserAction = (typeof USER_ACTIONS)[number]; type AnalysisRoute = { tag: "split"; plan: SplitPlan } | { tag: "single"; message: string }; @@ -75,7 +75,7 @@ class Commit { "Ready!", generateSplitPlan(this.providerConfig, diff, files, this.config.commit_convention, this.config.custom_template) ).chain((content) => this.followAnalysis(diff, files, content)) - : this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((message) => this.interact(diff, files, message)); + : this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((message) => this.interact(diff, message)); } private followAnalysis(diff: string, files: readonly string[], content: SplitPlanContent): Future { @@ -87,7 +87,7 @@ class Commit { case "split": return Split.fromResolved(this.config, this.providerConfig).runPlan(diff, files, route.plan, metadata); case "single": - return this.interact(diff, files, { text: route.message, metadata }); + return this.interact(diff, { text: route.message, metadata }); default: return absurd(route, "AnalysisRoute"); } @@ -95,14 +95,6 @@ class Commit { ); } - private startSplit(diff: string, files: readonly string[]): Future { - return loading( - "Generating split plan...", - "Split plan generated!", - generateSplitPlan(this.providerConfig, diff, files, this.config.commit_convention, this.config.custom_template) - ).chain((content) => Split.fromResolved(this.config, this.providerConfig).runPlan(diff, files, content.plan, content.metadata)); - } - diff(): Future { return repo.getStagedDiff(); } @@ -150,19 +142,17 @@ class Commit { ); } - interact(diff: string, files: readonly string[], generated: GeneratedContent): Future { - return this.promptAction(generated.text, files).chain((action) => { + interact(diff: string, generated: GeneratedContent): Future { + return this.promptAction(generated.text).chain((action) => { switch (action) { case "commit": return this.handleCommit(generated); case "commit_push": return this.handleCommitAndPush(generated); - case "split": - return this.startSplit(diff, files); case "regenerate": - return this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((msg) => this.interact(diff, files, msg)); + return this.generate(diff, this.config.commit_convention, this.config.custom_template).chain((msg) => this.interact(diff, msg)); case "adjust": - return this.handleAdjust(diff, files, generated); + return this.handleAdjust(diff, generated); case "cancel": return Future.resolve(undefined); } @@ -174,7 +164,7 @@ class Commit { return msg.includes("non-fast-forward") || msg.includes("updates were rejected"); } - private promptAction(message: string, files: readonly string[]): Future { + private promptAction(message: string): Future { return Future.attemptP(async () => { p.note(message, "Proposed Commit Message"); @@ -183,7 +173,6 @@ class Commit { options: [ { value: "commit_push" as const, label: "Commit & Push" }, { value: "commit" as const, label: "Commit" }, - ...(files.length >= 2 ? [{ value: "split" as const, label: "Split" }] : []), { value: "regenerate" as const, label: "Regenerate" }, { value: "adjust" as const, label: "Adjust" }, { value: "cancel" as const, label: "Cancel" } @@ -250,11 +239,11 @@ class Commit { }).chain((shouldForce) => (shouldForce ? this.push(request, undefined, false, true) : Future.resolve(undefined))); } - private handleAdjust(diff: string, files: readonly string[], generated: GeneratedContent): Future { + private handleAdjust(diff: string, generated: GeneratedContent): Future { return this.promptAdjustment().chain((maybeAdj) => maybeAdj instanceof Nothing ? - this.interact(diff, files, generated) - : this.refine(generated.text, maybeAdj.value, diff).chain((refined) => this.interact(diff, files, refined)) + this.interact(diff, generated) + : this.refine(generated.text, maybeAdj.value, diff).chain((refined) => this.interact(diff, refined)) ); } diff --git a/src/cli/split.ts b/src/cli/split.ts index 8018f89..7e6246d 100644 --- a/src/cli/split.ts +++ b/src/cli/split.ts @@ -5,11 +5,8 @@ import * as pr from "@/infra/github/pr"; import * as repo from "@/infra/git/repo"; import { Future } from "@/libs/future"; -import { loadConfig } from "@/infra/storage/config"; -import { Setup } from "@/cli/setup"; import { Commit } from "@/cli/commit"; import { type Config, type ProviderConfig } from "@/domain/config/config"; -import { resolveProvider } from "@/domain/llm/auth-resolver"; import { generateSplitPlan, type LlmRequestMetadata, type SplitPlanContent } from "@/domain/llm/router"; import { type SplitPlan } from "@/domain/split/plan"; import { Just, type Maybe } from "@/libs/maybe"; @@ -71,17 +68,6 @@ class Split { private readonly providerConfig: ProviderConfig ) {} - static create(): Future { - return loadConfig() - .chainRej((): Future => { - p.log.warn(color.yellow("No configuration found. Let's set you up first.")); - return Setup.create() - .chain((s) => s.run()) - .chain(() => loadConfig()); - }) - .chain((config) => resolveProvider(config).map((ai) => new Split(config, ai))); - } - static fromResolved(config: Config, providerConfig: ProviderConfig): Split { return new Split(config, providerConfig); } @@ -90,17 +76,6 @@ class Split { return this.interact(diff, files, plan, meta); } - run(): Future { - return repo - .checkIsGitRepo() - .chain(() => Future.concurrently({ diff: repo.getStagedDiff(), files: repo.listStagedPaths() })) - .chain(({ diff, files }) => this.generate(diff, files).chain((content) => this.interact(diff, files, content.plan, content.metadata))) - .mapRej((e) => { - p.log.error(color.red(e.message)); - return e; - }); - } - private generate(diff: string, files: readonly string[]): Future { return loading( "Generating split plan...", diff --git a/test/cli/commit.test.ts b/test/cli/commit.test.ts index ce46f61..aa92170 100644 --- a/test/cli/commit.test.ts +++ b/test/cli/commit.test.ts @@ -169,33 +169,6 @@ describe("Commit.run", () => { expect(router.generateCommitMessage).toHaveBeenCalled(); expect(repo.performCommit).toHaveBeenCalledWith("feat: generated"); }); - - it("forces a split plan when the user picks Split", async () => { - const repo = await import("@/infra/git/repo"); - vi.mocked(repo.listStagedPaths).mockReturnValue(Future.resolve(["a.ts", "b.ts"])); - const router = await import("@/domain/llm/router"); - vi.mocked(router.generateSplitPlan).mockReturnValue( - Future.resolve({ - plan: { - shouldSplit: true, - commits: [ - { message: "msg one", files: ["a.ts"] }, - { message: "msg two", files: ["b.ts"] } - ] - }, - metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } - }) - ); - const prompts = await import("@clack/prompts"); - vi.mocked(prompts.select).mockResolvedValueOnce("split").mockResolvedValueOnce("apply"); - - await runFuture(Commit.create().chain((c) => c.run())); - - expect(router.generateCommitMessage).toHaveBeenCalled(); - expect(router.generateSplitPlan).toHaveBeenCalled(); - expect(repo.performCommit).toHaveBeenNthCalledWith(1, "msg one", ["a.ts"]); - expect(repo.performCommit).toHaveBeenNthCalledWith(2, "msg two", ["b.ts"]); - }); }); describe("routeAnalysis", () => { diff --git a/test/cli/split.test.ts b/test/cli/split.test.ts index 9eff795..1903f87 100644 --- a/test/cli/split.test.ts +++ b/test/cli/split.test.ts @@ -13,33 +13,10 @@ import { Config } from "@/domain/config/config"; type ConfigValue = s.Infer; -vi.mock("@/infra/storage/config", () => ({ - loadConfig: vi.fn() -})); -vi.mock("@/domain/llm/auth-resolver", () => ({ - resolveProvider: vi.fn((c: ConfigValue) => Future.resolve(c.ai)) -})); vi.mock("@/infra/git/repo", () => ({ - checkIsGitRepo: vi.fn(() => Future.resolve(undefined)), - getStagedDiff: vi.fn(() => Future.resolve("staged diff")), - listStagedPaths: vi.fn(() => Future.resolve(["a.ts", "b.ts"])), performCommit: vi.fn(() => Future.resolve("\n 1 file changed\n")), findCommitMetadata: vi.fn() })); -vi.mock("@/domain/llm/router", () => ({ - generateSplitPlan: vi.fn(() => - Future.resolve({ - plan: { - shouldSplit: true, - commits: [ - { message: "msg one", files: ["a.ts"] }, - { message: "msg two", files: ["b.ts"] } - ] - }, - metadata: { durationMs: 1, model: { provider: "openai", model: "m", effort: "medium" }, tokens: Nothing() } - }) - ) -})); vi.mock("@clack/prompts", () => ({ note: vi.fn(), select: vi.fn(async () => "apply"), @@ -64,11 +41,24 @@ const config = (): ConfigValue => ({ ai: { provider: "openai", model: "gpt-4.1-mini", effort: Nothing(), auth_method: { type: "api_key", content: "sk" } } }); -describe("Split.run", () => { +const plan = { + shouldSplit: true, + commits: [ + { message: "msg one", files: ["a.ts"] }, + { message: "msg two", files: ["b.ts"] } + ] +}; + +const meta = { durationMs: 1, model: { provider: "openai" as const, model: "m", effort: "medium" as const }, tokens: Nothing() }; + +const runPlan = () => { + const cfg = config(); + return Split.fromResolved(cfg, cfg.ai).runPlan("staged diff", ["a.ts", "b.ts"], plan, meta); +}; + +describe("Split.runPlan", () => { beforeEach(async () => { vi.clearAllMocks(); - const storage = await import("@/infra/storage/config"); - vi.mocked(storage.loadConfig).mockReturnValue(Future.resolve(config())); const repo = await import("@/infra/git/repo"); vi.mocked(repo.findCommitMetadata).mockReturnValue( Future.resolve(Just({ hash: "h", short: "h", subject: "msg two", authorName: "t", authorEmail: "t@t.com", date: new Date() })) @@ -79,7 +69,7 @@ describe("Split.run", () => { }); it("applies each commit group with pathspecs when user selects apply", async () => { - await runFuture(Split.create().chain((s) => s.run())); + await runFuture(runPlan()); const repo = await import("@/infra/git/repo"); expect(repo.performCommit).toHaveBeenNthCalledWith(1, "msg one", ["a.ts"]); expect(repo.performCommit).toHaveBeenNthCalledWith(2, "msg two", ["b.ts"]); @@ -89,7 +79,7 @@ describe("Split.run", () => { const prompts = await import("@clack/prompts"); vi.mocked(prompts.select).mockResolvedValue("cancel"); - await runFuture(Split.create().chain((s) => s.run())); + await runFuture(runPlan()); const repo = await import("@/infra/git/repo"); expect(repo.performCommit).not.toHaveBeenCalled(); }); From 55665b055d1837602f396b24f2155a324cf40d14 Mon Sep 17 00:00:00 2001 From: rafaeelricco Date: Thu, 13 Aug 2026 08:54:28 -0300 Subject: [PATCH 41/41] Annotate split test metadata as LlmRequestMetadata --- test/cli/split.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/cli/split.test.ts b/test/cli/split.test.ts index 1903f87..c3638c7 100644 --- a/test/cli/split.test.ts +++ b/test/cli/split.test.ts @@ -10,6 +10,7 @@ import { Nothing, Just } from "@/libs/maybe"; import { runFuture } from "@test/helpers/run-future"; import * as s from "@/libs/json/schema"; import { Config } from "@/domain/config/config"; +import { type LlmRequestMetadata } from "@/domain/llm/router"; type ConfigValue = s.Infer; @@ -49,7 +50,11 @@ const plan = { ] }; -const meta = { durationMs: 1, model: { provider: "openai" as const, model: "m", effort: "medium" as const }, tokens: Nothing() }; +const meta: LlmRequestMetadata = { + durationMs: 1, + model: { provider: "openai", model: "m", effort: "medium" }, + tokens: Nothing() +}; const runPlan = () => { const cfg = config();