Add commit split for staged multi-commit plans - #36
Conversation
- Add `listStagedPaths` via `git diff --staged --name-only -z`. - Pass optional pathspecs to `performCommit` after `--`. - Run `git -C <worktree-root>` so root-relative pathspecs work from a subdirectory. - Cover leftover staged files and subdirectory pathspecs in the integration test.
- 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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59297560a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/domain/split/plan.ts">
<violation number="1" location="src/domain/split/plan.ts:17">
P2: When a staged filename starts or ends with whitespace, `nonEmptyString("file")` trims it and `validateSplitPlan` rejects a valid staged path as unknown. Decode file paths without trimming.</violation>
</file>
<file name="src/domain/commit/prompts.ts">
<violation number="1" location="src/domain/commit/prompts.ts:283">
P2: When a staged filename contains a newline, joining with `"\n"` changes one exact path into multiple lines in the prompt. The model then cannot return exact paths, and split-plan validation can reject the result as unknown paths. Serialize each path with JSON escaping instead of raw newline delimiting.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
- Persist `split_commits` on the config schema with a false default. - Update config fixtures and storage tests for the new field.
- Decode `should_split` and keep it through plan edits. - Extract JSON from prose and fold leftovers when not splitting.
- Prefer splitting unrelated layers and emit JSON only.
- Add `withMinEffort` and apply it in `generateSplitPlan`.
- Ask whether to analyze staged files and save `split_commits`.
- 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.
PR SummaryHigh Risk Overview Split planning adds Git layer is the heavy lift: README documents the setup toggle and auto-analysis behavior; tests cover routing, plan parsing, split apply/cancel, and isolated commits. Reviewed by Cursor Bugbot for commit 109ee97. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38d3e0b394
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 23 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/domain/split/plan.ts">
<violation number="1" location="src/domain/split/plan.ts:85">
P3: The `first === undefined` guard is unreachable. `splitPlanDecoder` (plan.ts:28) already rejects empty `commits` arrays with "expected at least 1 commit", so by the time `validateSplitPlan` runs, `plan.commits` is guaranteed non-empty and `first` is always defined. Drop the check and the duplicate error message.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 23 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 4 unresolved issues already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
- 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.
- 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.
- Type `nonEmptyCommits` so `D.object` infers `Decoder<SplitPlan>`. - Read OpenAI mock calls through `vi.mocked` in the router test.
- 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.
- Drop the last `<output_instructions>` 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.
- Slice from `{"should_split"` so a preview brace in surrounding prose cannot poison parse.
- Cover a schema-preview prefix before the real plan object.
- 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.
- Move path membership checks out of `validateSplitPlan` so lint:ci stays at the repo limit.
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
|
@cubic-dev-ai review this PR Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
@rafaeelricco I have started the AI code review. It will take a few minutes to complete. |
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
|
@cubic-dev-ai review this PR Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
@rafaeelricco I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55f082b5c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Invoke `sh` on the hook file so a `#!/bin/sh` shebang still runs without `git hook run`.
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
|
@cubic-dev-ai review this PR Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
@rafaeelricco I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cbea34d. Configure here.
- 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.
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
|
@cubic-dev-ai review this PR Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
@rafaeelricco I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 306a2c077b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- 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.
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 049cc4f68c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const env = /^#!\s*\/usr\/bin\/env(?:\s+(\S+))?/.exec(line); | ||
| if (env) return env[1] === undefined || env[1] === "" ? "sh" : lastPathSegment(env[1]); |
There was a problem hiding this comment.
On Windows with Git too old for git hook run, a valid hook beginning #!/usr/bin/env -S python3 -u makes this parser return -S, so the fallback attempts to spawn a program literally named -S and aborts the split; if this is a later group, earlier commits have already been applied. This is a P1 availability/correctness regression in the compatibility path. Parse env options and interpreter arguments, or delegate to a shebang-aware runner, so the actual interpreter and flags are invoked.
AGENTS.md reference: AGENTS.md:L7-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not applying. The old-Git Windows fallback already runs common #!/usr/bin/env python3 and #!/bin/sh hooks; env -S plus extra interpreter flags is not a realistic shebang on Git < 2.36.
Leaving open for a human call.
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/infra/git/repo.ts">
<violation number="1" location="src/infra/git/repo.ts:326">
P1: When a pre-commit script fails with its own `is not a git command` error, `isMissingGitHookCommand` treats it as a missing `git hook` subcommand and runs the hook again. Match only the specific `git: 'hook' is not a git command` message before triggering the fallback.</violation>
</file>
<file name="src/infra/git/parsers.ts">
<violation number="1" location="src/infra/git/parsers.ts:37">
P2: For a `#!/usr/bin/env -S node ...` hook shebang, `parseHookInterpreter` returns `-S` as the interpreter, so `runWindowsPreCommitFile` spawns `-S` (unknown command), the Future rejects, and the commit fails on the old-Git Windows fallback. The regex also drops any interpreter flags (e.g. `-u` in `#!/usr/bin/env python3 -u`). Parse the `env -S` remainder (or all args) instead of taking only the first token; when the first token is an env option like `-S` or `-u`, fall back to `sh` or skip the option rather than exec-ing it.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
|
|
||
| const isMissingGitHookCommand = (failure: CommandFailure): boolean => { | ||
| const text = `${failure.output.stderr}\n${failure.output.stdout}`.toLowerCase(); | ||
| return text.includes("is not a git command") && text.includes("hook"); |
There was a problem hiding this comment.
P1: When a pre-commit script fails with its own is not a git command error, isMissingGitHookCommand treats it as a missing git hook subcommand and runs the hook again. Match only the specific git: 'hook' is not a git command message before triggering the fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/infra/git/repo.ts, line 326:
<comment>When a pre-commit script fails with its own `is not a git command` error, `isMissingGitHookCommand` treats it as a missing `git hook` subcommand and runs the hook again. Match only the specific `git: 'hook' is not a git command` message before triggering the fallback.</comment>
<file context>
@@ -167,81 +164,220 @@ const hasExecutablePreCommit = (root: string): Future<Error, boolean> =>
- })
+const isMissingGitHookCommand = (failure: CommandFailure): boolean => {
+ const text = `${failure.output.stderr}\n${failure.output.stdout}`.toLowerCase();
+ return text.includes("is not a git command") && text.includes("hook");
+};
+
</file context>
| return text.includes("is not a git command") && text.includes("hook"); | |
| return text.includes("git: 'hook' is not a git command"); |
| 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]); |
There was a problem hiding this comment.
P2: For a #!/usr/bin/env -S node ... hook shebang, parseHookInterpreter returns -S as the interpreter, so runWindowsPreCommitFile spawns -S (unknown command), the Future rejects, and the commit fails on the old-Git Windows fallback. The regex also drops any interpreter flags (e.g. -u in #!/usr/bin/env python3 -u). Parse the env -S remainder (or all args) instead of taking only the first token; when the first token is an env option like -S or -u, fall back to sh or skip the option rather than exec-ing it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/infra/git/parsers.ts, line 37:
<comment>For a `#!/usr/bin/env -S node ...` hook shebang, `parseHookInterpreter` returns `-S` as the interpreter, so `runWindowsPreCommitFile` spawns `-S` (unknown command), the Future rejects, and the commit fails on the old-Git Windows fallback. The regex also drops any interpreter flags (e.g. `-u` in `#!/usr/bin/env python3 -u`). Parse the `env -S` remainder (or all args) instead of taking only the first token; when the first token is an env option like `-S` or `-u`, fall back to `sh` or skip the option rather than exec-ing it.</comment>
<file context>
@@ -28,6 +29,16 @@ const COMMIT_KEYS = ["hash", "short", "subject", "authorName", "authorEmail", "d
+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]);
</file context>
|
@codex review Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
|
@cubic-dev-ai review this PR Review this PR at HEAD ( Majors only (P0/P1): correctness bugs, security issues, data loss, broken Ignore completely: P3, nits, style, formatting, wording, optional refactors. Since last review:
If you find no major issues, say so in a clear line |
@rafaeelricco I have started the AI code review. It will take a few minutes to complete. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
2 existing issues remain and no new issues found across 31 files
Requires human review: Auto-approval blocked by 2 unresolved issues from a previous review of this commit.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
- 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.
- Drop the `split` CLI command from the parser, entrypoint, and alias targets. - Update README command docs to match.
- 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 109ee976ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| const runPlan = () => { | ||
| const cfg = config(); | ||
| return Split.fromResolved(cfg, cfg.ai).runPlan("staged diff", ["a.ts", "b.ts"], plan, meta); |
There was a problem hiding this comment.
pnpm typecheck fails with TS2345 because meta.tokens is inferred as Nothing<unknown>, which is incompatible with the required Maybe<TokenUsage> passed to runPlan; this directly breaks the repository's validation build. Give meta an explicit LlmRequestMetadata type or call Nothing<TokenUsage>() so the generic is inferred correctly.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
1 participant
Motivation
No motivation was provided.
What's New
Git Pathspecs
listStagedPathsreads staged paths withgit diff --staged --name-only -z.performCommittakes optional pathspecs so one staged set can become several commits.git -C <worktree-root>so those root-relative paths work when cwd is a subdirectory.Split Plan
Commit remaining staged changes.CLI
commit splitproposes a plan, then apply, apply and push, edit, move, reorder, regenerate, or cancel.Split Plan Flow
flowchart TB A["commit split"] --> B["Staged diff and paths"] B --> C["generateSplitPlan"] C --> D{"Action"} D -->|edit move reorder| D D -->|regenerate| C D -->|apply| E["performCommit per group"] D -->|apply and push| E E --> F["push publish or force-with-lease"] D -->|cancel| G["exit"]Changed Files
src/infra/git/repo.tslistStagedPaths,git -Crootsrc/domain/split/plan.tssrc/cli/split.tssrc/domain/commit/prompts.tsgetSplitPromptsrc/domain/llm/router.tsgenerateSplitPlansrc/cli/parser.tssplitcommandsrc/domain/alias/alias.tssplitindex.tssplitREADME.mdcommit splitTesting & Feedback
Try
commit spliton a mixed staged set, including from a subdirectory. Confirm apply leaves the right files in each commit, leftover paths get the remaining-changes commit, and cancel writes nothing.If you find any bugs or have recommendations for improvements, please open an issue and assign it to me.
Note
Cursor Bugbot is generating a summary for commit 5929756. Configure here.