t1049: Fix auto-rebase: abort stale rebase state before retrying#1480
t1049: Fix auto-rebase: abort stale rebase state before retrying#1480marcusquinn merged 1 commit intomainfrom
Conversation
When a prior rebase crashes mid-way (e.g., AI resolution timeout), it leaves rebase-merge/rebase-apply directories behind. Subsequent rebase attempts fail with 'already a rebase-merge directory' and the task stays permanently blocked. Fix: check for and abort stale rebase state before starting a new rebase. Uses git rev-parse --git-dir which works correctly for both worktrees and main repos.
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where a prior rebase operation crashing mid-way would leave behind stale rebase state directories. This state would then cause all subsequent rebase attempts to fail, permanently blocking associated tasks. The fix introduces a check to detect and abort any existing stale rebase state before proceeding with a new rebase, ensuring that tasks can recover and proceed as expected. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughA defensive cleanup step is added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Sat Feb 14 17:08:43 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a robust fix to prevent fatal errors from stale git rebase states. By checking for and aborting any existing rebase operations before starting a new one, it effectively addresses the issue of permanently blocked tasks. The implementation is clean and considers worktree environments correctly. I have one minor suggestion to improve logging and adherence to the repository's style guide regarding error redirection.
| pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)" | ||
| if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then |
There was a problem hiding this comment.
The use of 2>/dev/null on line 2181 to suppress potential errors from git rev-parse violates the repository style guide. The guide specifies that errors should be redirected to log files rather than being suppressed. This makes debugging more difficult if an unexpected issue occurs (e.g., if $git_dir is not a valid git repository).
A more robust approach is to redirect stderr to the supervisor log and explicitly check if a git directory was found before proceeding.
| pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)" | |
| if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then | |
| pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>>"$SUPERVISOR_LOG")" || true | |
| if [[ -n "$pre_git_state_dir" && ( -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ) ]]; then |
References
- The style guide at line 50 states that
2>/dev/nullis only acceptable when redirecting to log files, not for blanket suppression of errors. The suggested change redirects stderr to the supervisor log to comply with this rule. (link)
![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
🤖 Fix all issues with AI agents
In @.agents/scripts/supervisor/deploy.sh:
- Around line 2178-2185: The stale-rebase detection uses
pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir)" which can return a
relative path (".git") and thus the subsequent -d checks run against the
supervisor CWD; make the git-dir absolute (either by calling git -C "$git_dir"
rev-parse --absolute-git-dir or by normalizing pre_git_state_dir after
rev-parse: if it does not start with '/', prefix it with "$git_dir/") so the
rebase-merge/rebase-apply existence checks operate on the correct repository
directory; apply the same normalization to the other occurrences that use
rev-parse --git-dir (the similar checks after the rebase is initiated).
| # t1049: Abort any stale rebase state from a prior crash before starting | ||
| # a new rebase. Without this, git refuses with "already a rebase-merge directory". | ||
| local pre_git_state_dir | ||
| pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)" | ||
| if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then | ||
| log_warn "rebase_sibling_pr: aborting stale rebase state for $task_id" | ||
| git -C "$git_dir" rebase --abort 2>>"$SUPERVISOR_LOG" || true | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and examine its structure
if [ -f ".agents/scripts/supervisor/deploy.sh" ]; then
echo "=== File found ==="
wc -l ".agents/scripts/supervisor/deploy.sh"
else
echo "File not found at expected path"
find . -name "deploy.sh" -type f 2>/dev/null | head -5
fiRepository: marcusquinn/aidevops
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Examine the specific lines mentioned in the review
if [ -f ".agents/scripts/supervisor/deploy.sh" ]; then
echo "=== Lines 2178-2185 ==="
sed -n '2178,2185p' ".agents/scripts/supervisor/deploy.sh" | cat -n
echo ""
echo "=== Context: Lines 2170-2210 ==="
sed -n '2170,2210p' ".agents/scripts/supervisor/deploy.sh" | cat -n
fiRepository: marcusquinn/aidevops
Length of output: 3058
🏁 Script executed:
#!/bin/bash
# Verify git rev-parse behavior with a temporary test repo
tmpdir=$(mktemp -d)
trap 'rm -rf "$tmpdir"' EXIT
# Create a test repo
git init "$tmpdir/testrepo" >/dev/null 2>&1
echo "=== Non-worktree case ==="
result=$(git -C "$tmpdir/testrepo" rev-parse --git-dir 2>/dev/null)
echo "Result: '$result'"
if [[ "$result" == /* ]]; then
echo "Status: Absolute path (unexpected for regular repo)"
else
echo "Status: Relative path (as claimed in review)"
fi
# Create an initial commit and add worktree
cd "$tmpdir/testrepo" && git commit --allow-empty -m "init" >/dev/null 2>&1
git worktree add "$tmpdir/wt" -b testbranch >/dev/null 2>&1
echo ""
echo "=== Worktree case ==="
result=$(git -C "$tmpdir/wt" rev-parse --git-dir 2>/dev/null)
echo "Result: '$result'"
if [[ "$result" == /* ]]; then
echo "Status: Absolute path (as claimed in review)"
else
echo "Status: Relative path (contradicts review claim)"
fi
# Test the actual path resolution behavior
echo ""
echo "=== Path resolution behavior test ==="
cd "$tmpdir" || exit 1
non_worktree_gitdir=$(git -C "$tmpdir/testrepo" rev-parse --git-dir 2>/dev/null)
echo "From testrepo: '$non_worktree_gitdir'"
# Test if -d check works with the relative path
if [[ -d "$non_worktree_gitdir" ]]; then
echo "Relative path check PASSES from current dir"
else
echo "Relative path check FAILS from current dir"
fi
# Test if -d check works with constructed absolute path
if [[ -d "$tmpdir/testrepo/$non_worktree_gitdir" ]]; then
echo "Constructed absolute path check PASSES"
else
echo "Constructed absolute path check FAILS"
fiRepository: marcusquinn/aidevops
Length of output: 375
🏁 Script executed:
#!/bin/bash
# Find where $git_dir is initialized in the rebase_sibling_pr function
sed -n '2150,2180p' ".agents/scripts/supervisor/deploy.sh" | cat -nRepository: marcusquinn/aidevops
Length of output: 1214
🏁 Script executed:
#!/bin/bash
# Search for git_dir assignments in the function context
rg 'git_dir=' ".agents/scripts/supervisor/deploy.sh" -B 2 -A 2 | head -50Repository: marcusquinn/aidevops
Length of output: 370
🏁 Script executed:
#!/bin/bash
# Look for the function definition and early variable assignments
sed -n '2100,2180p' ".agents/scripts/supervisor/deploy.sh" | grep -E 'rebase_sibling_pr|git_dir|use_worktree' | cat -nRepository: marcusquinn/aidevops
Length of output: 748
🏁 Script executed:
#!/bin/bash
# Check for other instances of the same pattern at lines 2197-2198 and 2234-2235
echo "=== Lines 2195-2200 ==="
sed -n '2195,2200p' ".agents/scripts/supervisor/deploy.sh" | cat -n
echo ""
echo "=== Lines 2230-2240 ==="
sed -n '2230,2240p' ".agents/scripts/supervisor/deploy.sh" | cat -nRepository: marcusquinn/aidevops
Length of output: 1356
🏁 Script executed:
#!/bin/bash
# Verify that git_dir can indeed be either trepo or tworktree at line 2178
# and test what rev-parse returns for an actual worktree
tmpdir=$(mktemp -d)
trap 'rm -rf "$tmpdir"' EXIT
git init "$tmpdir/main_repo" >/dev/null 2>&1
cd "$tmpdir/main_repo"
git commit --allow-empty -m "init" >/dev/null 2>&1
git worktree add "$tmpdir/worktree_path" -b test-branch >/dev/null 2>&1
echo "=== Test 1: Main repo with -C ==="
main_git_dir=$(git -C "$tmpdir/main_repo" rev-parse --git-dir 2>/dev/null)
echo "Result: '$main_git_dir'"
echo "Is absolute: $(if [[ \"$main_git_dir\" == /* ]]; then echo 'YES'; else echo 'NO'; fi)"
echo ""
echo "=== Test 2: Worktree with -C ==="
wt_git_dir=$(git -C "$tmpdir/worktree_path" rev-parse --git-dir 2>/dev/null)
echo "Result: '$wt_git_dir'"
echo "Is absolute: $(if [[ \"$wt_git_dir\" == /* ]]; then echo 'YES'; else echo 'NO'; fi)"
echo ""
echo "=== Test 3: Directory existence check from different CWD ==="
cd "$tmpdir" || exit 1
# Simulate the bug: check with relative path from wrong CWD
if [[ -d "$main_git_dir" ]]; then
echo "Check '-d \"$main_git_dir\"' from $tmpdir: FOUND (unexpected!)"
else
echo "Check '-d \"$main_git_dir\"' from $tmpdir: NOT FOUND (confirms bug)"
fi
# Fixed version: prepend the repo path
fixed_path="$tmpdir/main_repo/$main_git_dir"
if [[ -d "$fixed_path" ]]; then
echo "Check '-d \"$fixed_path\"': FOUND (fix works)"
else
echo "Check '-d \"$fixed_path\"': NOT FOUND"
fiRepository: marcusquinn/aidevops
Length of output: 387
Relative path from rev-parse --git-dir breaks stale rebase detection for non-worktree repos.
When git_dir points to the main repository (non-worktree case at this stage), git -C "$git_dir" rev-parse --git-dir returns .git—a relative path. The subsequent -d checks then resolve against the supervisor script's CWD rather than $git_dir, causing the stale rebase state detection to silently fail.
This directly breaks the crash-recovery feature for the primary (non-worktree) use case. The fix requires normalizing relative paths:
🐛 Construct absolute path for git directory
local pre_git_state_dir
pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)"
+ # rev-parse --git-dir returns a relative path (e.g., ".git") for non-worktree repos;
+ # make it absolute so the -d checks resolve correctly.
+ if [[ "$pre_git_state_dir" != /* ]]; then
+ pre_git_state_dir="$git_dir/$pre_git_state_dir"
+ fi
if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then
log_warn "rebase_sibling_pr: aborting stale rebase state for $task_id"
git -C "$git_dir" rebase --abort 2>>"$SUPERVISOR_LOG" || true
fiNote: The same pattern exists at lines 2197–2198 and 2234–2235 (both in conditional branches after the rebase is initiated), though those are pre-existing.
📝 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.
| # t1049: Abort any stale rebase state from a prior crash before starting | |
| # a new rebase. Without this, git refuses with "already a rebase-merge directory". | |
| local pre_git_state_dir | |
| pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)" | |
| if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then | |
| log_warn "rebase_sibling_pr: aborting stale rebase state for $task_id" | |
| git -C "$git_dir" rebase --abort 2>>"$SUPERVISOR_LOG" || true | |
| fi | |
| # t1049: Abort any stale rebase state from a prior crash before starting | |
| # a new rebase. Without this, git refuses with "already a rebase-merge directory". | |
| local pre_git_state_dir | |
| pre_git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)" | |
| # rev-parse --git-dir returns a relative path (e.g., ".git") for non-worktree repos; | |
| # make it absolute so the -d checks resolve correctly. | |
| if [[ "$pre_git_state_dir" != /* ]]; then | |
| pre_git_state_dir="$git_dir/$pre_git_state_dir" | |
| fi | |
| if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then | |
| log_warn "rebase_sibling_pr: aborting stale rebase state for $task_id" | |
| git -C "$git_dir" rebase --abort 2>>"$SUPERVISOR_LOG" || true | |
| fi |
🤖 Prompt for AI Agents
In @.agents/scripts/supervisor/deploy.sh around lines 2178 - 2185, The
stale-rebase detection uses pre_git_state_dir="$(git -C "$git_dir" rev-parse
--git-dir)" which can return a relative path (".git") and thus the subsequent -d
checks run against the supervisor CWD; make the git-dir absolute (either by
calling git -C "$git_dir" rev-parse --absolute-git-dir or by normalizing
pre_git_state_dir after rev-parse: if it does not start with '/', prefix it with
"$git_dir/") so the rebase-merge/rebase-apply existence checks operate on the
correct repository directory; apply the same normalization to the other
occurrences that use rev-parse --git-dir (the similar checks after the rebase is
initiated).
1 participant
Summary
rebase-merge/rebase-applystate before starting a new rebase inrebase_sibling_pr()Root Cause
Follow-up to t1048. When a prior rebase crashes mid-way (AI resolution timeout, process kill, etc.), it leaves rebase state directories behind. Every subsequent rebase attempt on that worktree fails immediately with
fatal: It seems that there is already a rebase-merge directory, and the task stays permanently blocked.Fix
Before calling
git rebase origin/main, check if stale rebase state exists and abort it. The check runs once, before the worktree/non-worktree branch, so it covers both paths. Usesgit rev-parse --git-dirwhich resolves correctly for worktrees.Testing
bash -n: passshellcheck -S warning -x: no new warningsSummary by CodeRabbit