Skip to content

t1049: Fix auto-rebase: abort stale rebase state before retrying #1480

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.

Sign up for GitHub

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

Merged
marcusquinn merged 1 commit into from
Feb 14, 2026
Merged

t1049: Fix auto-rebase: abort stale rebase state before retrying #1480

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .agents/scripts/supervisor/deploy.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2175,6 +2175,15 @@ rebase_sibling_pr() {
return 1
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)"
if [[ -d "$pre_git_state_dir/rebase-merge" || -d "$pre_git_state_dir/rebase-apply" ]]; then
Comment on lines +2181 to +2182
Copy link

@gemini-code-assist gemini-code-assist bot Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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
  1. The style guide at line 50 states that 2>/dev/null is 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)

log_warn "rebase_sibling_pr: aborting stale rebase state for $task_id"
git -C "$git_dir" rebase --abort 2>>"$SUPERVISOR_LOG" || true
fi
Comment on lines +2178 to +2185
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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
fi

Repository: 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
fi

Repository: 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"
fi

Repository: 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 -n

Repository: 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 -50

Repository: 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 -n

Repository: 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 -n

Repository: 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"
fi

Repository: 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
 	fi

Note: 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.

Suggested change
# 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).


if [[ "$use_worktree" == "true" ]]; then
# Worktree is already on the branch — rebase in place
if ! git -C "$git_dir" rebase origin/main 2>>"$SUPERVISOR_LOG"; then
Expand Down
Loading