feat(new-change): add --branch flag to auto-create and checkout a git branch#939
Conversation
… branch Adds an optional --branch flag to the `openspec new change <name>` command that, when provided, automatically creates and checks out a new git branch named `openspec/<change-name>` after the change directory is created. ## What changed ### src/utils/git-utils.ts (new file) - `isGitRepository(cwd)` — detects git repos via `git rev-parse --git-dir`, returns false gracefully when not in a git repo or git is not installed - `createAndCheckoutBranch(cwd, branchName)` — runs `git checkout -b` via `execFileSync` (no shell, cross-platform safe on macOS/Linux/Windows) - `getBranchNameForChange(changeName)` — derives branch name as `openspec/<changeName>`, namespacing branches for easy identification in git tooling (GitLens, GitHub, etc.) ### src/commands/workflow/new-change.ts - Added `branch?: boolean` to `NewChangeOptions` interface - After successful change directory creation, when `--branch` is set: 1. Verifies the working directory is a git repository 2. Creates and checks out `openspec/<change-name>` branch 3. Shows a dedicated ora spinner for the git step - On git failure (not a git repo, branch already exists, git not on PATH), shows an `ora().warn()` message and sets `process.exitCode = 1`, but does NOT roll back the already-created change directory ### src/cli/index.ts - Registered `--branch` option on the `new change` command - Commander.js automatically passes it through to `newChangeCommand` ### test/core/commands/new-change-git-branch.test.ts (new file) - Unit tests for `isGitRepository`: true on success, false when throws - Unit tests for `createAndCheckoutBranch`: verifies correct `execFileSync` args and propagates errors correctly - Unit tests for `getBranchNameForChange`: correct `openspec/<name>` pattern - Integration tests for `newChangeCommand`: - No git calls when `--branch` is absent - Git utilities called and change dir created when `--branch` is true - `exitCode=1` set when not in a git repo (change dir still created) - `exitCode=1` set when branch already exists (change dir still created) - macOS symlink path resolution (`/var` → `/private/var`) handled in setup ### openspec/changes/propose-git-branch/ (new OpenSpec change) - proposal.md, design.md, specs/, tasks.md — full spec-driven change artifacts documenting the what, why, and how of this feature ## Design decisions - Branch name always `openspec/<change-name>` — no custom override - Uses Node.js built-in `child_process.execFileSync` (no new dependencies) - Change directory creation is the primary operation; git is a side-effect and failures are non-fatal to the change creation itself Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughA new optional Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant NewChangeCommand
participant FileSystem
participant GitUtils
participant Git
User->>CLI: openspec new change <name> --branch
CLI->>NewChangeCommand: invoke with branch=true
NewChangeCommand->>FileSystem: create change directory
NewChangeCommand->>NewChangeCommand: emit success message
alt branch flag is true
NewChangeCommand->>GitUtils: isGitRepository(cwd)
GitUtils->>Git: git rev-parse --git-dir
Git-->>GitUtils: success/failure
alt is git repository
NewChangeCommand->>GitUtils: getBranchNameForChange(name)
GitUtils-->>NewChangeCommand: openspec/<name>
NewChangeCommand->>GitUtils: createAndCheckoutBranch(cwd, branchName)
GitUtils->>Git: git checkout -b openspec/<name>
Git-->>GitUtils: success/error
alt branch created
GitUtils-->>NewChangeCommand: success
NewChangeCommand->>User: branch created and checked out
else branch exists or git unavailable
GitUtils-->>NewChangeCommand: throws error
NewChangeCommand->>User: emit warning, exit code 1
end
else not a git repository
NewChangeCommand->>User: emit warning, exit code 1
end
else branch flag is false
NewChangeCommand->>User: done (no git operations)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/core/commands/new-change-git-branch.test.ts (2)
122-122: Unused variableoraOutput.This array is declared but never populated or asserted against. If spinner output verification is not needed, remove it; otherwise, implement the capture logic.
🧹 Remove unused variable
let originalCwd: string; let originalExitCode: number | undefined; - let oraOutput: string[]; beforeEach(async () => { ... - oraOutput = []; isGitRepositoryMock.mockReset();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/commands/new-change-git-branch.test.ts` at line 122, The test declares an unused array oraOutput; remove this dead declaration or implement spinner capture logic: either delete the let oraOutput: string[] declaration from new-change-git-branch.test.ts, or if you intend to assert spinner output, initialize and populate oraOutput by mocking the spinner (where the test uses ora/spinner mocks) and push spinner messages into oraOutput before asserting; update assertions to reference oraOutput accordingly.
45-54: Tests verify the mock, not the real implementation.The
getBranchNameForChangeimported at line 35 is the mocked version (fromgetBranchNameForChangeMockat line 22). These tests pass because the mock is hardcoded to returnopenspec/${name}, not because the real function works correctly.Unlike the
isGitRepositoryandcreateAndCheckoutBranchtests which usevi.importActualto test the real implementations, this test provides no actual coverage forgetBranchNameForChange.♻️ Suggested fix using vi.importActual
describe('getBranchNameForChange', () => { + // Import the real implementation bypassing the vi.mock + const { getBranchNameForChange: realGetBranchNameForChange } = await vi.importActual< + typeof import('../../../src/utils/git-utils.js') + >('../../../src/utils/git-utils.js'); + it('returns openspec/<changeName>', () => { - // Use the real function by importing from the mocked module with passthrough - expect(getBranchNameForChange('my-feature')).toBe('openspec/my-feature'); + expect(realGetBranchNameForChange('my-feature')).toBe('openspec/my-feature'); }); it('works for multi-segment change names', () => { - expect(getBranchNameForChange('add-user-auth')).toBe('openspec/add-user-auth'); + expect(realGetBranchNameForChange('add-user-auth')).toBe('openspec/add-user-auth'); }); });Alternatively, convert to an async describe block like the other unit tests (lines 60, 88) to use
vi.importActual.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/commands/new-change-git-branch.test.ts` around lines 45 - 54, The tests are exercising the mocked getBranchNameForChangeMock rather than the real implementation; update the test to import the actual function before assertions (either convert the describe block to an async describe and use const { getBranchNameForChange } = await vi.importActual('...') or call vi.importActual inside the it blocks) so that getBranchNameForChange (not getBranchNameForChangeMock) is the real implementation under test; replace usages of the mock with the real getBranchNameForChange and keep assertions the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/workflow/new-change.ts`:
- Around line 72-79: The branch creation error handling in the block that sets
`message` (used to decide warnings via `branchSpinner.warn`) only inspects
`error.message`, which is brittle because `execFileSync(..., { stdio: 'pipe' })`
in `git-utils.ts` surfaces process stderr on the thrown error as `error.stderr`;
update the logic that computes `message` (the variable around
`branchSpinner.warn` and references to `branchName`) to coalesce `error.message`
and the stringified `error.stderr` (handle Buffer -> string) into one diagnostic
string and then use that combined string to test for "already exists", "not
found", "ENOENT" etc., so classification uses actual git stderr as well as the
JS message.
---
Nitpick comments:
In `@test/core/commands/new-change-git-branch.test.ts`:
- Line 122: The test declares an unused array oraOutput; remove this dead
declaration or implement spinner capture logic: either delete the let oraOutput:
string[] declaration from new-change-git-branch.test.ts, or if you intend to
assert spinner output, initialize and populate oraOutput by mocking the spinner
(where the test uses ora/spinner mocks) and push spinner messages into oraOutput
before asserting; update assertions to reference oraOutput accordingly.
- Around line 45-54: The tests are exercising the mocked
getBranchNameForChangeMock rather than the real implementation; update the test
to import the actual function before assertions (either convert the describe
block to an async describe and use const { getBranchNameForChange } = await
vi.importActual('...') or call vi.importActual inside the it blocks) so that
getBranchNameForChange (not getBranchNameForChangeMock) is the real
implementation under test; replace usages of the mock with the real
getBranchNameForChange and keep assertions the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e46a0794-645b-4490-a5cd-e2cdbf29725d
📒 Files selected for processing (9)
openspec/changes/propose-git-branch/.openspec.yamlopenspec/changes/propose-git-branch/design.mdopenspec/changes/propose-git-branch/proposal.mdopenspec/changes/propose-git-branch/specs/new-change-git-branch/spec.mdopenspec/changes/propose-git-branch/tasks.mdsrc/cli/index.tssrc/commands/workflow/new-change.tssrc/utils/git-utils.tstest/core/commands/new-change-git-branch.test.ts
| const message = (error as Error).message ?? String(error); | ||
| if (message.includes('already exists')) { | ||
| branchSpinner.warn(`Branch '${branchName}' already exists`); | ||
| } else if (message.includes('not found') || message.includes('ENOENT')) { | ||
| branchSpinner.warn(`Branch not created: git not found on PATH`); | ||
| } else { | ||
| branchSpinner.warn(`Branch not created: ${message}`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js v20+, for child_process.execFileSync('git', args, { stdio: 'pipe' }), when the command exits non-zero, is diagnostic output expected in error.stderr rather than reliably in error.message?
💡 Result:
Yes. With execFileSync(..., { stdio: 'pipe' }), when the child exits non‑zero Node throws an Error that includes the full spawnSync() result, i.e. you should expect the captured output to be available on err.stdout / err.stderr (as Buffers unless you set encoding). Node’s docs explicitly describe handling the failure case by reading const { stdout, stderr } = err; rather than relying on err.message. [1]
err.message is just a formatted diagnostic string (e.g., “Command failed: …”) and is not specified to reliably contain the process’s stderr output, so treating err.stderr as the authoritative diagnostic output is the correct/expected approach. [1][2]
Sources:
[1] Node.js docs: child_process.execFileSync() error includes full spawnSync() result; example reads err.stdout/err.stderr. (nodejs.org)
[2] Discussion noting stderr is available via spawnSync/sync error properties rather than “catching” a message string. (stackoverflow.com)
Citations:
- 1: https://nodejs.org/api/child_process.html
- 2: https://stackoverflow.com/questions/57484453/how-to-get-err-stderr-from-execsync?utm_source=openai
🏁 Script executed:
# First, locate and read the file to understand the context around lines 72-79
cat -n src/commands/workflow/new-change.ts | sed -n '50,90p'Repository: Fission-AI/OpenSpec
Length of output: 1926
🏁 Script executed:
# Search for the createAndCheckoutBranch function definition
rg -n "createAndCheckoutBranch" src/commands/workflow/new-change.ts -A 2 -B 2Repository: Fission-AI/OpenSpec
Length of output: 551
🏁 Script executed:
# Find git-utils.js
find src -name "git-utils.js" -o -name "git-utils.ts"Repository: Fission-AI/OpenSpec
Length of output: 85
🏁 Script executed:
# Read the git-utils.ts file to find createAndCheckoutBranch implementation
cat -n src/utils/git-utils.tsRepository: Fission-AI/OpenSpec
Length of output: 1152
Use error.stderr for git failure classification, not only error.message.
With execFileSync(..., { stdio: 'pipe' }) at line 21 of git-utils.ts, when git exits non-zero, Node.js throws an error with the full spawnSync() result including error.stderr (as a Buffer). The current error handling at lines 72–79 only checks error.message, which is a formatted diagnostic string not specified to reliably contain the process's stderr output. This makes detection of "already exists" and "git not found" brittle and prone to misfiring.
Combine error.message and error.stderr to build a comprehensive diagnostic string for error classification:
Suggested fix
} catch (error) {
- const message = (error as Error).message ?? String(error);
- if (message.includes('already exists')) {
+ const err = error as NodeJS.ErrnoException & { stderr?: Buffer | string };
+ const message = err.message ?? String(error);
+ const stderrText = Buffer.isBuffer(err.stderr) ? err.stderr.toString('utf8') : (err.stderr ?? '');
+ const diagnostic = `${message}\n${stderrText}`.toLowerCase();
+
+ if (diagnostic.includes('already exists')) {
branchSpinner.warn(`Branch '${branchName}' already exists`);
- } else if (message.includes('not found') || message.includes('ENOENT')) {
+ } else if (diagnostic.includes('not found') || diagnostic.includes('enoent')) {
branchSpinner.warn(`Branch not created: git not found on PATH`);
} else {
- branchSpinner.warn(`Branch not created: ${message}`);
+ branchSpinner.warn(`Branch not created: ${stderrText || message}`);
}
process.exitCode = 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const message = (error as Error).message ?? String(error); | |
| if (message.includes('already exists')) { | |
| branchSpinner.warn(`Branch '${branchName}' already exists`); | |
| } else if (message.includes('not found') || message.includes('ENOENT')) { | |
| branchSpinner.warn(`Branch not created: git not found on PATH`); | |
| } else { | |
| branchSpinner.warn(`Branch not created: ${message}`); | |
| } | |
| const err = error as NodeJS.ErrnoException & { stderr?: Buffer | string }; | |
| const message = err.message ?? String(error); | |
| const stderrText = Buffer.isBuffer(err.stderr) ? err.stderr.toString('utf8') : (err.stderr ?? ''); | |
| const diagnostic = `${message}\n${stderrText}`.toLowerCase(); | |
| if (diagnostic.includes('already exists')) { | |
| branchSpinner.warn(`Branch '${branchName}' already exists`); | |
| } else if (diagnostic.includes('not found') || diagnostic.includes('enoent')) { | |
| branchSpinner.warn(`Branch not created: git not found on PATH`); | |
| } else { | |
| branchSpinner.warn(`Branch not created: ${stderrText || message}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/workflow/new-change.ts` around lines 72 - 79, The branch
creation error handling in the block that sets `message` (used to decide
warnings via `branchSpinner.warn`) only inspects `error.message`, which is
brittle because `execFileSync(..., { stdio: 'pipe' })` in `git-utils.ts`
surfaces process stderr on the thrown error as `error.stderr`; update the logic
that computes `message` (the variable around `branchSpinner.warn` and references
to `branchName`) to coalesce `error.message` and the stringified `error.stderr`
(handle Buffer -> string) into one diagnostic string and then use that combined
string to test for "already exists", "not found", "ENOENT" etc., so
classification uses actual git stderr as well as the JS message.
1 participant
Summary
--branchflag toopenspec new change <name>that automatically creates and checks out a git branch namedopenspec/<change-name>after the change directory is createdsrc/utils/git-utils.tswith three cross-platform helpers (isGitRepository,createAndCheckoutBranch,getBranchNameForChange) using Node.js built-inchild_process.execFileSync— no new dependenciesMotivation
When starting a new change, developers currently have to manually run
git checkout -b <branch>as a second step. The--branchflag makes this a single command:What Changed
New file:
src/utils/git-utils.tsisGitRepository(cwd)— detects git repos viagit rev-parse --git-dircreateAndCheckoutBranch(cwd, branchName)— runsgit checkout -bviaexecFileSyncgetBranchNameForChange(changeName)— always returnsopenspec/<changeName>Modified:
src/commands/workflow/new-change.tsbranch?: booleantoNewChangeOptionsModified:
src/cli/index.ts--branchflag on thenew changecommandNew file:
test/core/commands/new-change-git-branch.test.tsisGitRepository(true/false),createAndCheckoutBranch(correct args, throws on failure),getBranchNameForChange(naming convention), andnewChangeCommandintegration (flag absent, success, not-git-repo, branch-exists)New:
openspec/changes/propose-git-branch/Design Decisions
openspec/<change-name>main/feature/etc.execFileSyncdirectly (no lib)git rev-parse --git-dirfor detectionTest Plan
pnpm build— no TypeScript errorspnpm test— all 1351 tests pass (10 new tests added)openspec new change test-branch-feature --branchcreated the change and checked outopenspec/test-branch-featureexecFileSyncwith git binary directly (no shell expansion)Summary by CodeRabbit
Release Notes
--branchflag to theopenspec new changecommand. When enabled, the command automatically creates and checks out a git branch namedopenspec/<change-name>after creating the change directory, with graceful error handling for non-git repositories and existing branches.