t1072, t1073: Multi-commit rebase loop + sequential subtask dispatch#1558
t1072, t1073: Multi-commit rebase loop + sequential subtask dispatch#1558marcusquinn merged 1 commit intomainfrom
Conversation
…1073) t1072: Add _resolve_rebase_loop() to deploy.sh — wraps the AI conflict resolution + git rebase --continue cycle in a while loop (max 10 iterations) so branches with multiple conflicting commits are fully rebased instead of aborting after the first conflict. Replaces the duplicated single-shot resolution logic in both worktree and non-worktree code paths. t1073: Add subtask sibling sequencing to cmd_next() in state.sh — when selecting queued tasks for dispatch, skip subtasks (IDs like t1063.2) if any earlier sibling (t1063.1) is still in a non-terminal state. This prevents parallel dispatch of subtasks that modify the same files, eliminating the merge conflicts that caused 5 tech-stack PRs to stall. Subtasks now dispatch sequentially by suffix order automatically.
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 introduces critical fixes to the supervisor's task management and Git rebase capabilities. It resolves issues where multi-commit branches would fail to rebase correctly after the first conflict, and where related subtasks would cause conflicts by being dispatched in parallel. These changes significantly improve the robustness of automated PR merging and prevent recurring merge conflicts for interdependent tasks, unblocking several existing PRs and ensuring smoother future operations. 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
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request centralizes AI-assisted rebase conflict resolution into a dedicated loop function and adds filtering to prevent parallel execution of interdependent subtasks, improving reliability of multi-commit rebases and task dependency handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
✨ 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: Tue Feb 17 01:21:53 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 two important fixes: a robust rebase loop for multi-commit branches and a sequential dispatch mechanism for subtasks to prevent merge conflicts. The changes are well-structured, particularly the refactoring in deploy.sh which deduplicates the rebase logic.
My review includes a few suggestions for improvement:
- In
deploy.sh, there's a minor violation of the shell style guide and an opportunity to improve efficiency by moving a repeated command out of a loop. - In
state.sh, I've identified a potential N+1 query performance issue and a risky error suppression pattern that could mask database problems.
Addressing these points will enhance the robustness and maintainability of the new logic.
| AND id != '$(sql_escape "$cid")' | ||
| AND CAST(REPLACE(id, '$(sql_escape "$parent_id").', '') AS INTEGER) < $suffix | ||
| AND status NOT IN ('verified','cancelled','deployed','complete'); | ||
| " 2>/dev/null || echo "0") |
There was a problem hiding this comment.
Suppressing stderr with 2>/dev/null and defaulting to 0 with || echo "0" is risky. It can mask underlying database errors (e.g., a malformed query, DB corruption), causing the script to proceed with incorrect assumptions. This could lead to subtasks being dispatched out of order. This also violates the repository style guide (line 50), which prohibits blanket error suppression. It would be safer to let the script fail on a database error so the problem can be investigated.
References
2>/dev/nullis acceptable ONLY when redirecting to log files, not blanket suppression (link)
| local git_dir="$1" | ||
| local task_id="$2" |
There was a problem hiding this comment.
This function violates the repository's shell script style guide. Line 11 requires declaring local variables and assigning values from positional parameters in separate statements to ensure exit code safety (e.g., local var; var="$1").
| local git_dir="$1" | |
| local task_id="$2" | |
| local git_dir; git_dir="$1" | |
| local task_id; task_id="$2" |
References
- Use
local var="$1"pattern in functions (declare and assign separately for exit code safety) (link)
| local git_state_dir | ||
| git_state_dir="$(git -C "$git_dir" rev-parse --git-dir 2>/dev/null)" |
There was a problem hiding this comment.
The git_state_dir variable is calculated on each loop iteration by calling git rev-parse --git-dir. Since the .git directory path is constant within the scope of this function, this call can be moved outside and before the while loop. This would improve efficiency by avoiding redundant process execution and also remove the need for the second identical call on line 2174.
| while IFS=$'\t' read -r cid crepo cdesc cmodel; do | ||
| [[ "$count" -ge "$limit" ]] && break | ||
|
|
||
| # Check if this is a subtask (contains a dot) | ||
| if [[ "$cid" == *.* ]]; then | ||
| local parent_id="${cid%.*}" | ||
| local suffix="${cid##*.}" | ||
|
|
||
| # Check if any earlier sibling (same parent, lower suffix) is non-terminal | ||
| local earlier_active | ||
| earlier_active=$(db "$SUPERVISOR_DB" " | ||
| SELECT count(*) FROM tasks | ||
| WHERE id LIKE '$(sql_escape "$parent_id").%' | ||
| AND id != '$(sql_escape "$cid")' | ||
| AND CAST(REPLACE(id, '$(sql_escape "$parent_id").', '') AS INTEGER) < $suffix | ||
| AND status NOT IN ('verified','cancelled','deployed','complete'); | ||
| " 2>/dev/null || echo "0") | ||
|
|
||
| if [[ "$earlier_active" -gt 0 ]]; then | ||
| log_info " cmd_next: deferring $cid — earlier sibling(s) of $parent_id still active" | ||
| continue | ||
| fi | ||
| fi | ||
|
|
||
| printf '%s\t%s\t%s\t%s\n' "$cid" "$crepo" "$cdesc" "$cmodel" | ||
| count=$((count + 1)) | ||
| done <<<"$candidates" |
There was a problem hiding this comment.
This while loop executes a database query for every candidate subtask. This is a classic N+1 query problem and can be inefficient, especially if the number of candidate tasks is large. Consider fetching the status of all potentially relevant sibling tasks in a single query before the loop, then performing the filtering logic in memory within the loop. This would significantly reduce the number of database round-trips.
1 participant
Summary
Two fixes that address the root cause of 5 tech-stack PRs being permanently stuck in merge-conflict blocked state:
t1072: Multi-commit rebase loop
rebase_sibling_pr()resolved conflicts for the first commit, calledgit rebase --continue, but if the next commit also conflicted, it aborted the entire rebase_resolve_rebase_loop()— a while loop (max 10 iterations) that keeps resolving conflicts and continuing until the rebase completes or a resolution failst1073: Sequential subtask dispatch
tech-stack-helper.sh,tech-stack-lookup.md), so when the first merged, all siblings conflictedcmd_next()now checks if a queued subtask has earlier siblings still in non-terminal states. If so, it defers the subtask until the earlier sibling completes. Subtasks dispatch sequentially by suffix order automaticallyt1063.%)Impact
Closes #1556
Closes #1557
Summary by CodeRabbit