-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(new-change): add --branch flag to auto-create and checkout a git branch #939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
alargham-abdalla-delivery-hero
wants to merge
1
commit into
Fission-AI:main
Choose a base branch
from
alargham-abdalla-delivery-hero:feat/propose-git-branch
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-04-08 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| ## Context | ||
|
|
||
| The `openspec new change <name>` command creates a new change directory but leaves the developer on their current git branch. Developers then manually run `git checkout -b <branch>` to isolate their work. This is a recurring two-step friction point in the workflow. | ||
|
|
||
| The codebase already uses Node.js built-in `child_process` (`execSync`, `execFileSync`) for running external commands (see `src/commands/feedback.ts`). No third-party git library is present. The CLI is Commander.js-based (TypeScript, ESM, Node ≥20.19.0) and must run on macOS, Linux, and Windows. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
| - Add `--branch` boolean flag to `openspec new change <name>` | ||
| - When `--branch` is set, create and checkout a git branch named `openspec/<change-name>` after the change directory is created | ||
| - Fail clearly if the working directory is not a git repository | ||
| - Fail clearly if the branch already exists | ||
| - The change directory creation still succeeds even if git branch creation fails (git step is last) | ||
|
|
||
| **Non-Goals:** | ||
| - Pushing the branch to remote | ||
| - Automatically creating a branch without the explicit `--branch` flag | ||
| - Supporting a custom branch name override (the name is always derived from the change name) | ||
| - Modifying `git config` or any git settings | ||
|
|
||
| ## Decisions | ||
|
|
||
| ### Use `execFileSync` with `git` binary directly | ||
|
|
||
| **Decision**: Run git commands via `execFileSync('git', args, { cwd: projectRoot, stdio: 'pipe' })`. | ||
|
|
||
| **Rationale**: Consistent with existing pattern in `src/commands/feedback.ts`. `execFileSync` avoids shell injection risks and works cross-platform (Windows, macOS, Linux) as long as `git` is on PATH. No new dependencies needed. | ||
|
|
||
| **Alternative considered**: `simple-git` or `execa` — rejected to avoid adding a dependency for what amounts to two git commands. | ||
|
|
||
| ### Branch naming: `openspec/<change-name>` | ||
|
|
||
| **Decision**: Branch name is always `openspec/<change-name>` (e.g., `openspec/propose-git-branch`). | ||
|
|
||
| **Rationale**: Using a namespace prefix groups openspec-related branches in git tooling (GitLens, GitHub, etc.) and avoids conflicts with common branch names like `main`, `feature`, `fix`. The `/` is a standard git branch namespace separator. | ||
|
|
||
| **Alternative considered**: Using the raw change name (e.g., `propose-git-branch`) — rejected because it risks collisions with existing branch names and provides no namespace context. | ||
|
|
||
| ### Git detection: `git rev-parse --git-dir` | ||
|
|
||
| **Decision**: Before creating the branch, verify the working directory is inside a git repo using `execFileSync('git', ['rev-parse', '--git-dir'], ...)`. If it throws, the directory is not a git repo. | ||
|
|
||
| **Rationale**: Simple, fast, cross-platform. Returns `.git` (or the git dir path) on success and exits nonzero on failure. | ||
|
|
||
| ### Change creation first, git operation second | ||
|
|
||
| **Decision**: Create the change directory first, then perform the git branch operation. If git fails, the change directory already exists (acceptable). | ||
|
|
||
| **Rationale**: Change directory creation is the primary operation. Git branching is a convenience side-effect. A failed git step should not roll back a successful change creation — the user can create the branch manually. | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **git not on PATH** → Clear error: "git not found. Please ensure git is installed and available on PATH." Unlikely in practice but worth handling. | ||
| - **Branch already exists** → Clear error: `Branch 'openspec/<name>' already exists`. User must delete it manually. | ||
| - **Detached HEAD state** → `git checkout -b` still works in detached HEAD, so this is not a blocker. | ||
| - **Windows path separator in branch name** → Branch name is derived from the kebab-case change name (already validated, only `[a-z0-9-]`), so no slashes or backslashes appear in the name part. The `openspec/` prefix uses a forward slash which git handles correctly on all platforms (git always uses forward slashes for branch namespaces). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| ## Why | ||
|
|
||
| When starting a new change, developers typically also want to work on an isolated git branch for that change. Currently, users must manually create and checkout a git branch after running `openspec new change`, which adds friction to the workflow start. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - Add an optional `--branch` flag to the `openspec new change` command | ||
| - When `--branch` is provided, automatically create and checkout a git branch named after the change (e.g., `openspec/propose-git-branch`) | ||
| - If the branch already exists, fail with a clear error message | ||
| - If the working directory is not a git repository, fail with a clear error message | ||
| - Works cross-platform (macOS, Linux, Windows) using Node.js child process instead of shell-specific commands | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| - `new-change-git-branch`: Optional `--branch` flag for `openspec new change` that creates and checks out a git branch named after the change, enabling a one-step workflow start. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| <!-- No existing spec-level requirement changes. The change-creation spec covers programmatic API; this feature adds a CLI flag and git side-effect which is a new capability. --> | ||
|
|
||
| ## Impact | ||
|
|
||
| - `src/commands/workflow/new-change.ts`: Add branch creation logic and git operations | ||
| - `src/cli/index.ts`: Add `--branch` flag to the `new change` command definition | ||
| - New tests for the git branch creation feature | ||
| - Cross-platform: uses Node.js `child_process` with `git` binary (available on all platforms where git is installed) |
81 changes: 81 additions & 0 deletions
81
openspec/changes/propose-git-branch/specs/new-change-git-branch/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| # new-change-git-branch Specification | ||
|
|
||
| ## Purpose | ||
| Define the optional `--branch` flag for `openspec new change` that creates and checks out a git branch named after the change. | ||
|
|
||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Git Branch Creation Flag | ||
|
|
||
| The `openspec new change` command SHALL support an optional `--branch` flag that, when provided, creates and checks out a new git branch named `openspec/<change-name>` after the change directory is created. | ||
|
|
||
| #### Scenario: Branch created and checked out on success | ||
|
|
||
| - **WHEN** executing `openspec new change my-feature --branch` | ||
| - **AND** the current directory is inside a git repository | ||
| - **AND** the branch `openspec/my-feature` does not already exist | ||
| - **THEN** the system creates the change directory at `openspec/changes/my-feature/` | ||
| - **AND** creates a new git branch named `openspec/my-feature` | ||
| - **AND** checks out the new branch | ||
| - **AND** displays a success message indicating the branch was created and checked out | ||
|
|
||
| #### Scenario: No branch created when flag is absent | ||
|
|
||
| - **WHEN** executing `openspec new change my-feature` without `--branch` | ||
| - **THEN** the system creates the change directory normally | ||
| - **AND** does NOT create or checkout any git branch | ||
|
|
||
| #### Scenario: Error when not in a git repository | ||
|
|
||
| - **WHEN** executing `openspec new change my-feature --branch` | ||
| - **AND** the current directory is NOT inside a git repository | ||
| - **THEN** the change directory is still created successfully | ||
| - **AND** the system outputs a warning indicating git is not available or the directory is not a git repository | ||
| - **AND** the process exits with a non-zero exit code | ||
|
|
||
| #### Scenario: Error when branch already exists | ||
|
|
||
| - **WHEN** executing `openspec new change my-feature --branch` | ||
| - **AND** the branch `openspec/my-feature` already exists in the git repository | ||
| - **THEN** the change directory is still created successfully | ||
| - **AND** the system outputs an error indicating the branch already exists | ||
| - **AND** the process exits with a non-zero exit code | ||
|
|
||
| #### Scenario: Error when git is not installed | ||
|
|
||
| - **WHEN** executing `openspec new change my-feature --branch` | ||
| - **AND** `git` is not available on the system PATH | ||
| - **THEN** the change directory is still created successfully | ||
| - **AND** the system outputs an error indicating git was not found | ||
| - **AND** the process exits with a non-zero exit code | ||
|
|
||
| ### Requirement: Branch Name Derived from Change Name | ||
|
|
||
| The git branch name SHALL be deterministically derived from the change name using the pattern `openspec/<change-name>`. | ||
|
|
||
| #### Scenario: Branch name uses openspec namespace | ||
|
|
||
| - **WHEN** creating a change named `add-user-auth` with `--branch` | ||
| - **THEN** the created branch name is exactly `openspec/add-user-auth` | ||
|
|
||
| #### Scenario: Branch name inherits change name validation | ||
|
|
||
| - **GIVEN** change names are already validated to be kebab-case (`[a-z0-9][a-z0-9-]*[a-z0-9]`) | ||
| - **WHEN** the `--branch` flag is used | ||
| - **THEN** the resulting branch name `openspec/<change-name>` is always a valid git branch name | ||
|
|
||
| ### Requirement: Cross-Platform Git Execution | ||
|
|
||
| The system SHALL execute git commands using `execFileSync` with the `git` binary directly, without relying on shell-specific features. | ||
|
|
||
| #### Scenario: Git commands run without shell on all platforms | ||
|
|
||
| - **WHEN** the `--branch` flag is used on any supported platform (macOS, Linux, Windows) | ||
| - **THEN** git is invoked via `execFileSync('git', [...args], { cwd: projectRoot, stdio: 'pipe' })` | ||
| - **AND** no shell expansion or shell-specific syntax is used | ||
|
|
||
| #### Scenario: Git repository detection uses rev-parse | ||
|
|
||
| - **WHEN** the `--branch` flag is used | ||
| - **THEN** the system checks for a git repository by running `git rev-parse --git-dir` | ||
| - **AND** treats a non-zero exit code as "not a git repository" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| ## 1. Core Git Branch Utility | ||
|
|
||
| - [x] 1.1 Create `src/utils/git-utils.ts` with `isGitRepository(cwd: string): boolean` function using `execFileSync('git', ['rev-parse', '--git-dir'], { cwd, stdio: 'pipe' })` | ||
| - [x] 1.2 Add `createAndCheckoutBranch(cwd: string, branchName: string): void` function to `src/utils/git-utils.ts` that runs `git checkout -b <branchName>` via `execFileSync` | ||
| - [x] 1.3 Add `getBranchNameForChange(changeName: string): string` helper that returns `openspec/<changeName>` | ||
| - [x] 1.4 Export all new functions from `src/utils/git-utils.ts` | ||
|
|
||
| ## 2. New Change Command Integration | ||
|
|
||
| - [x] 2.1 Add `branch?: boolean` to `NewChangeOptions` interface in `src/commands/workflow/new-change.ts` | ||
| - [x] 2.2 After successful change creation, check `options.branch` and if truthy call git utility functions | ||
| - [x] 2.3 Show spinner step for git branch creation (e.g., "Creating branch 'openspec/my-feature'...") | ||
| - [x] 2.4 On git failure (not a git repo, branch exists, git not found), show warning via `ora().warn()` and exit with code 1 | ||
|
|
||
| ## 3. CLI Flag Registration | ||
|
|
||
| - [x] 3.1 Add `.option('--branch', 'Create and checkout a git branch named openspec/<change-name>')` to the `new change` command in `src/cli/index.ts` | ||
| - [x] 3.2 Pass `branch` option through to `newChangeCommand` call | ||
|
|
||
| ## 4. Tests | ||
|
|
||
| - [x] 4.1 Create `test/core/commands/new-change-git-branch.test.ts` with unit tests for `isGitRepository`, `createAndCheckoutBranch`, and `getBranchNameForChange` using mocked `execFileSync` | ||
| - [x] 4.2 Test: `isGitRepository` returns true when git command succeeds | ||
| - [x] 4.3 Test: `isGitRepository` returns false when git command throws (not a git repo or git not found) | ||
| - [x] 4.4 Test: `getBranchNameForChange('my-feature')` returns `'openspec/my-feature'` | ||
| - [x] 4.5 Test: `createAndCheckoutBranch` calls `execFileSync` with correct args `['checkout', '-b', 'openspec/my-feature']` | ||
| - [x] 4.6 Test: `newChangeCommand` with `--branch` calls git utilities when change creation succeeds | ||
| - [x] 4.7 Test: `newChangeCommand` with `--branch` exits nonzero when not in git repo (but change dir is still created) | ||
| - [x] 4.8 Test: `newChangeCommand` without `--branch` never calls git utilities | ||
|
|
||
| ## 5. Verification | ||
|
|
||
| - [x] 5.1 Run `pnpm build` and ensure no TypeScript errors | ||
| - [x] 5.2 Run `pnpm test` and ensure all tests pass | ||
| - [x] 5.3 Manually test: `openspec new change test-branch-feature --branch` creates the branch and checks it out | ||
| - [x] 5.4 Verify with `git branch` that `openspec/test-branch-feature` is the current branch | ||
| - [x] 5.5 Clean up test branch: `git checkout main && git branch -D openspec/test-branch-feature` | ||
| - [x] 5.6 Delete the test change: `rm -rf openspec/changes/test-branch-feature` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { execFileSync } from 'child_process'; | ||
|
|
||
| /** | ||
| * Check if the given directory is inside a git repository. | ||
| * Uses `git rev-parse --git-dir` which exits nonzero when not in a git repo. | ||
| */ | ||
| export function isGitRepository(cwd: string): boolean { | ||
| try { | ||
| execFileSync('git', ['rev-parse', '--git-dir'], { cwd, stdio: 'pipe' }); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Create a new git branch and check it out. | ||
| * Throws if the branch already exists or git is not available. | ||
| */ | ||
| export function createAndCheckoutBranch(cwd: string, branchName: string): void { | ||
| execFileSync('git', ['checkout', '-b', branchName], { cwd, stdio: 'pipe' }); | ||
| } | ||
|
|
||
| /** | ||
| * Derive the git branch name for a given change name. | ||
| * Always returns `openspec/<changeName>`. | ||
| */ | ||
| export function getBranchNameForChange(changeName: string): string { | ||
| return `openspec/${changeName}`; | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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 anErrorthat includes the fullspawnSync()result, i.e. you should expect the captured output to be available onerr.stdout/err.stderr(asBuffers unless you setencoding). Node’s docs explicitly describe handling the failure case by readingconst { stdout, stderr } = err;rather than relying onerr.message. [1]err.messageis just a formatted diagnostic string (e.g., “Command failed: …”) and is not specified to reliably contain the process’s stderr output, so treatingerr.stderras the authoritative diagnostic output is the correct/expected approach. [1][2]Sources:
[1] Node.js docs:
child_process.execFileSync()error includes fullspawnSync()result; example readserr.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:
🏁 Script executed:
Repository: Fission-AI/OpenSpec
Length of output: 1926
🏁 Script executed:
Repository: Fission-AI/OpenSpec
Length of output: 551
🏁 Script executed:
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.stderrfor git failure classification, not onlyerror.message.With
execFileSync(..., { stdio: 'pipe' })at line 21 ofgit-utils.ts, when git exits non-zero, Node.js throws an error with the fullspawnSync()result includingerror.stderr(as aBuffer). The current error handling at lines 72–79 only checkserror.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.messageanderror.stderrto 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
🤖 Prompt for AI Agents