-
Notifications
You must be signed in to change notification settings - Fork 513
refactor: replace git command for branch retrieval #371
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,7 +59,7 @@ export function createCommitHandler() { | |||||||||||||||||||||||||||||
| const commitHash = hashOutput.trim().substring(0, 8); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Get branch name | ||||||||||||||||||||||||||||||
| const { stdout: branchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||||||||||||||||||||||||||||||
| const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', { | ||||||||||||||||||||||||||||||
| cwd: worktreePath, | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| const branchName = branchOutput.trim(); | ||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling for detached HEAD state. In a detached HEAD state, 🔎 Proposed fix to add error handling- // Get branch name
- const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', {
- cwd: worktreePath,
- });
- const branchName = branchOutput.trim();
+ // Get branch name (may be null in detached HEAD state)
+ let branchName: string | null = null;
+ try {
+ const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', {
+ cwd: worktreePath,
+ });
+ branchName = branchOutput.trim();
+ } catch {
+ // Detached HEAD state - branch name unavailable
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,7 +43,7 @@ export function createCreatePRHandler() { | |
| const effectiveProjectPath = projectPath || worktreePath; | ||
|
|
||
| // Get current branch name | ||
| const { stdout: branchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||
| const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's examine the file and context around line 46
cat -n apps/server/src/routes/worktree/routes/create-pr.ts | sed -n '30,60p'Repository: AutoMaker-Org/automaker Length of output: 1204 🏁 Script executed: # Run the provided search for detached HEAD handling
rg -n -C3 'detached.HEAD|HEAD detached' --type=tsRepository: AutoMaker-Org/automaker Length of output: 49 🏁 Script executed: # Also search for git symbolic-ref usage in the codebase
rg -n 'git symbolic-ref' --type=tsRepository: AutoMaker-Org/automaker Length of output: 1503 🏁 Script executed: # Search for error handling around the git command in the file
cat -n apps/server/src/routes/worktree/routes/create-pr.ts | head -100Repository: AutoMaker-Org/automaker Length of output: 4097 🏁 Script executed: # View the rest of the create-pr.ts file to see error handling
cat -n apps/server/src/routes/worktree/routes/create-pr.ts | sed -n '100,150p'Repository: AutoMaker-Org/automaker Length of output: 2128 🏁 Script executed: # Check the common.ts to understand execAsync error behavior
cat -n apps/server/src/routes/worktree/routes/common.ts | head -80Repository: AutoMaker-Org/automaker Length of output: 147 🏁 Script executed: # Check if there's worktree initialization that ensures non-detached HEAD
rg -n -C5 'worktree create|worktree add' --type=tsRepository: AutoMaker-Org/automaker Length of output: 4737 🏁 Script executed: # Find common.ts in the worktree routes
find apps/server/src/routes/worktree -name "common.ts"Repository: AutoMaker-Org/automaker Length of output: 108 🏁 Script executed: # Check the error handling wrapper that execAsync would trigger
cat -n apps/server/src/routes/worktree/common.ts | head -100Repository: AutoMaker-Org/automaker Length of output: 4136 🏁 Script executed: # Look at the try-catch block in create-pr.ts to see what happens when execAsync throws
cat -n apps/server/src/routes/worktree/routes/create-pr.ts | sed -n '150,200p'Repository: AutoMaker-Org/automaker Length of output: 2516 🏁 Script executed: # Check the final catch block of the create-pr.ts handler to see generic error handling
cat -n apps/server/src/routes/worktree/routes/create-pr.ts | tail -50Repository: AutoMaker-Org/automaker Length of output: 2008 Add explicit detached HEAD error handling. The
The current implicit error handling via the generic catch block is insufficient for user guidance. 🤖 Prompt for AI Agents |
||
| cwd: worktreePath, | ||
| env: execEnv, | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,7 +34,7 @@ export function createListBranchesHandler() { | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Get current branch | ||||||||||||||||||||||||||||||||||||
| const { stdout: currentBranchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||||||||||||||||||||||||||||||||||||
| const { stdout: currentBranchOutput } = await execAsync('git symbolic-ref --short HEAD', { | ||||||||||||||||||||||||||||||||||||
| cwd: worktreePath, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
| const currentBranch = currentBranchOutput.trim(); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+37
to
40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Check if the file exists and examine the relevant lines
if [ -f "apps/server/src/routes/worktree/routes/list-branches.ts" ]; then
echo "=== File exists, showing context around lines 37-40 ==="
wc -l apps/server/src/routes/worktree/routes/list-branches.ts
echo ""
echo "=== Lines 30-50 ==="
sed -n '30,50p' apps/server/src/routes/worktree/routes/list-branches.ts
else
echo "File not found at specified path"
fiRepository: AutoMaker-Org/automaker Length of output: 938 🏁 Script executed: # Search for similar git symbolic-ref patterns in the codebase
echo "=== Searching for git symbolic-ref usage ==="
rg "git symbolic-ref" --type ts --type tsx -nRepository: AutoMaker-Org/automaker Length of output: 156 🏁 Script executed: # Search for git rev-parse usage to understand common patterns
echo "=== Searching for git rev-parse usage ==="
rg "git rev-parse" --type ts --type tsx -n -A 2Repository: AutoMaker-Org/automaker Length of output: 153 🏁 Script executed: # Search for git symbolic-ref and git rev-parse with correct syntax
echo "=== Searching for git symbolic-ref usage ==="
rg "git symbolic-ref" --type ts -n
echo ""
echo "=== Searching for git rev-parse usage ==="
rg "git rev-parse" --type ts -n -A 2Repository: AutoMaker-Org/automaker Length of output: 4748 🏁 Script executed: # Examine the full function to understand its purpose and callers
echo "=== Full function context (lines 1-60) ==="
sed -n '1,60p' apps/server/src/routes/worktree/routes/list-branches.tsRepository: AutoMaker-Org/automaker Length of output: 1977 🏁 Script executed: # Check if there are error handlers elsewhere in the codebase for execAsync
echo "=== Searching for execAsync error handling patterns ==="
rg "execAsync" --type ts -A 3 | head -50Repository: AutoMaker-Org/automaker Length of output: 2952 Add graceful handling for detached HEAD state. The Proposed fix- const { stdout: currentBranchOutput } = await execAsync('git symbolic-ref --short HEAD', {
- cwd: worktreePath,
- });
- const currentBranch = currentBranchOutput.trim();
+ let currentBranch: string;
+ try {
+ const { stdout: currentBranchOutput } = await execAsync('git symbolic-ref --short HEAD', {
+ cwd: worktreePath,
+ });
+ currentBranch = currentBranchOutput.trim();
+ } catch {
+ // Detached HEAD state - use commit SHA
+ const { stdout: commitSha } = await execAsync('git rev-parse --short HEAD', {
+ cwd: worktreePath,
+ });
+ currentBranch = `HEAD (detached at ${commitSha.trim()})`;
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,7 +35,7 @@ export function createMergeHandler() { | |
| const worktreePath = path.join(projectPath, '.worktrees', featureId); | ||
|
|
||
| // Get current branch | ||
| const { stdout: currentBranch } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||
| const { stdout: currentBranch } = await execAsync('git symbolic-ref --short HEAD', { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| cwd: projectPath, | ||
| }); | ||
|
Comment on lines
+38
to
40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n apps/server/src/routes/worktree/routes/merge.tsRepository: AutoMaker-Org/automaker Length of output: 2860 Remove unused Lines 38-40 fetch the current branch name but never reference it in the merge logic. The command will throw an error if the repository is in a detached HEAD state, with no error handling. Delete these lines entirely since the merge operation does not depend on knowing the current branch. 🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,7 +28,7 @@ export function createPullHandler() { | |||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Get current branch name | ||||||||||||||||||||||||||||||||||||||
| const { stdout: branchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||||||||||||||||||||||||||||||||||||||
| const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', { | ||||||||||||||||||||||||||||||||||||||
| cwd: worktreePath, | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| const branchName = branchOutput.trim(); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+31
to
34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling for detached HEAD state. When a repo is in a detached HEAD state, 🔎 Proposed fix to add error handling- // Get current branch name
- const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', {
- cwd: worktreePath,
- });
- const branchName = branchOutput.trim();
+ // Get current branch name
+ let branchName: string;
+ try {
+ const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', {
+ cwd: worktreePath,
+ });
+ branchName = branchOutput.trim();
+ } catch {
+ res.status(400).json({
+ success: false,
+ error: 'Cannot pull: worktree is in detached HEAD state',
+ });
+ return;
+ }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,7 @@ export function createPushHandler() { | |
| } | ||
|
|
||
| // Get branch name | ||
| const { stdout: branchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||
| const { stdout: branchOutput } = await execAsync('git symbolic-ref --short HEAD', { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat -n apps/server/src/routes/worktree/routes/push.ts | head -80Repository: AutoMaker-Org/automaker Length of output: 2256 🏁 Script executed: #!/bin/bash
# Create a temporary test repository
TEST_DIR=$(mktemp -d)
cd "$TEST_DIR"
git init
# Create initial commit
echo "test" > file.txt
git add file.txt
git commit -m "initial"
# Get the commit hash
COMMIT_HASH=$(git rev-parse HEAD)
# Test 1: Normal branch state
echo "=== Test 1: Normal branch state ==="
echo "git symbolic-ref --short HEAD:"
git symbolic-ref --short HEAD
echo "git rev-parse --abbrev-ref HEAD:"
git rev-parse --abbrev-ref HEAD
# Test 2: Detached HEAD state
echo ""
echo "=== Test 2: Detached HEAD state ==="
git checkout "$COMMIT_HASH"
echo "git symbolic-ref --short HEAD:"
git symbolic-ref --short HEAD 2>&1 || echo "FAILED with exit code: $?"
echo "git rev-parse --abbrev-ref HEAD:"
git rev-parse --abbrev-ref HEAD 2>&1
# Cleanup
cd /
rm -rf "$TEST_DIR"Repository: AutoMaker-Org/automaker Length of output: 1868 🏁 Script executed: #!/bin/bash
# Create a temporary test repository
TEST_DIR=$(mktemp -d)
cd "$TEST_DIR"
git init
# Configure git
git config user.email "[email protected]"
git config user.name "Test User"
# Create initial commit
echo "test" > file.txt
git add file.txt
git commit -m "initial"
# Get the commit hash
COMMIT_HASH=$(git rev-parse HEAD)
# Test 1: Normal branch state
echo "=== Test 1: Normal branch state ==="
echo "git symbolic-ref --short HEAD:"
git symbolic-ref --short HEAD
echo "Exit code: $?"
echo ""
echo "git rev-parse --abbrev-ref HEAD:"
git rev-parse --abbrev-ref HEAD
echo "Exit code: $?"
# Test 2: Detached HEAD state
echo ""
echo "=== Test 2: Detached HEAD state ==="
echo "Checking out commit: $COMMIT_HASH"
git checkout "$COMMIT_HASH"
echo ""
echo "Verifying detached HEAD:"
git symbolic-ref -q HEAD && echo "Still on branch" || echo "In detached HEAD state"
echo ""
echo "git symbolic-ref --short HEAD:"
git symbolic-ref --short HEAD 2>&1
echo "Exit code: $?"
echo ""
echo "git rev-parse --abbrev-ref HEAD:"
git rev-parse --abbrev-ref HEAD 2>&1
echo "Exit code: $?"
# Cleanup
cd /
rm -rf "$TEST_DIR"Repository: AutoMaker-Org/automaker Length of output: 1829 Add error handling for detached HEAD state.
Note: |
||
| cwd: worktreePath, | ||
| }); | ||
| const branchName = branchOutput.trim(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -87,7 +87,7 @@ export function createSwitchBranchHandler() { | |||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Get current branch | ||||||||||||||||||||||||||||||
| const { stdout: currentBranchOutput } = await execAsync('git rev-parse --abbrev-ref HEAD', { | ||||||||||||||||||||||||||||||
| const { stdout: currentBranchOutput } = await execAsync('git symbolic-ref --short HEAD', { | ||||||||||||||||||||||||||||||
| cwd: worktreePath, | ||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||
| const previousBranch = currentBranchOutput.trim(); | ||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling for detached HEAD state. When in a detached HEAD state, 🔎 Proposed fix to add error handling- // Get current branch
- const { stdout: currentBranchOutput } = await execAsync('git symbolic-ref --short HEAD', {
- cwd: worktreePath,
- });
- const previousBranch = currentBranchOutput.trim();
+ // Get current branch (may be in detached HEAD state)
+ let previousBranch: string;
+ try {
+ const { stdout: currentBranchOutput } = await execAsync('git symbolic-ref --short HEAD', {
+ cwd: worktreePath,
+ });
+ previousBranch = currentBranchOutput.trim();
+ } catch {
+ previousBranch = 'detached HEAD';
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
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
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 190
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 3121
🏁 Script executed:
rg -n 'detached|symbolic-ref|previousBranch' apps/server/src/routes/worktree/ --type=tsRepository: AutoMaker-Org/automaker
Length of output: 1996
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 700
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 5583
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 1503
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 3028
🏁 Script executed:
cat -n apps/server/src/routes/worktree/middleware.ts | head -100Repository: AutoMaker-Org/automaker
Length of output: 2746
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 613
🏁 Script executed:
cat -n apps/server/tests/integration/routes/worktree/create.integration.test.ts | head -150Repository: AutoMaker-Org/automaker
Length of output: 2489
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 49
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 7421
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 96
🏁 Script executed:
Repository: AutoMaker-Org/automaker
Length of output: 69
Handle detached HEAD state across symbolic-ref calls.
The
git symbolic-ref --short HEADcall will fail when the repository is in a detached HEAD state, even though the repository passes all middleware validation (which only checks that commits exist viagit rev-parse --verify HEAD). This affects 10 files in the worktree routes and causes ungraceful 500 errors with generic messages instead of informative feedback.For
checkout-branch.tsspecifically: creating a branch from detached HEAD is a valid Git operation (git checkout -bworks fine), but retrieving the previous branch state fails. Consider wrapping thegit symbolic-refcall in a try-catch to either:previousBranchto the commit SHA (viagit rev-parse HEAD) when detachedThis pattern should be addressed consistently across all symbolic-ref calls in the worktree route handlers.
🤖 Prompt for AI Agents