From c3aef27aa692a105ae8ca7a468a94cc82433b439 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 30 Apr 2026 22:07:46 -0400 Subject: [PATCH] fix(#2916): fail-fast on switch/checkout, gate fork-point warning to fresh branches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CodeRabbit findings on PR #2921 (review 4209533909 + comment 3171721073, both still unresolved): A. Branch switch and create steps now abort on non-zero exit. Previously `git switch "$BRANCH_NAME"` and `git checkout -b "$BRANCH_NAME" "origin/$DEFAULT_BRANCH"` could fail (locked worktree, dirty tree refusing the checkout, etc.) and the workflow would silently continue on the wrong branch — sending the phase's later commits to the wrong place. Both calls now `|| { echo "ERROR: …" >&2; exit 1; }`. B. The fork-point base-warning is now scoped to the creation arm of the if/else. Previously it ran for the resume path too, so a legitimate resumed branch where origin/$DEFAULT_BRANCH had advanced since first creation would falsely warn ("does not fork from origin/"). Moving the check inside the else arm means it only runs immediately after a fresh `git checkout -b`, when the merge-base check is meaningful. Same fix mirrored in get-shit-done/workflows/quick.md. execute-phase.md stays at the 1700-line XL budget. Full suite: 6102/6102. --- get-shit-done/workflows/execute-phase.md | 15 ++++++++------- get-shit-done/workflows/quick.md | 15 +++++++++------ 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/get-shit-done/workflows/execute-phase.md b/get-shit-done/workflows/execute-phase.md index f7e88cbc4..86d243c6a 100644 --- a/get-shit-done/workflows/execute-phase.md +++ b/get-shit-done/workflows/execute-phase.md @@ -226,7 +226,7 @@ DEFAULT_BRANCH=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/de DEFAULT_BRANCH=${DEFAULT_BRANCH:-main} if git show-ref --verify --quiet "refs/heads/$BRANCH_NAME"; then - git switch "$BRANCH_NAME" + git switch "$BRANCH_NAME" || { echo "ERROR: Could not switch to existing branch '$BRANCH_NAME'." >&2; exit 1; } else if ! git fetch --quiet origin "$DEFAULT_BRANCH"; then # #2916 git show-ref --verify --quiet "refs/remotes/origin/$DEFAULT_BRANCH" \ @@ -238,12 +238,13 @@ else else git switch --quiet "$DEFAULT_BRANCH" 2>/dev/null && git merge --ff-only --quiet "origin/$DEFAULT_BRANCH" 2>/dev/null || true fi - git checkout -b "$BRANCH_NAME" "origin/$DEFAULT_BRANCH" # pinned base (#2916) -fi - -# Warn only when HEAD did NOT fork from origin/$DEFAULT_BRANCH (merge-base ≠ tip) — #2916. -if MB=$(git merge-base HEAD "origin/${DEFAULT_BRANCH}" 2>/dev/null) && DT=$(git rev-parse --verify --quiet "refs/remotes/origin/${DEFAULT_BRANCH}" 2>/dev/null) && [ "$MB" != "$DT" ]; then - echo "WARNING: Phase branch '$BRANCH_NAME' does not fork from origin/${DEFAULT_BRANCH}; verify the base is intentional." + git checkout -b "$BRANCH_NAME" "origin/$DEFAULT_BRANCH" \ + || { echo "ERROR: Could not create '$BRANCH_NAME' from origin/$DEFAULT_BRANCH (#2916)." >&2; exit 1; } + # Warn only on fresh creation when HEAD did NOT fork from origin/$DEFAULT_BRANCH — #2916. + # Skipped on resume because origin may have advanced legitimately since first creation. + if MB=$(git merge-base HEAD "origin/${DEFAULT_BRANCH}" 2>/dev/null) && DT=$(git rev-parse --verify --quiet "refs/remotes/origin/${DEFAULT_BRANCH}" 2>/dev/null) && [ "$MB" != "$DT" ]; then + echo "WARNING: Phase branch '$BRANCH_NAME' does not fork from origin/${DEFAULT_BRANCH}; verify the base is intentional." + fi fi ``` diff --git a/get-shit-done/workflows/quick.md b/get-shit-done/workflows/quick.md index d3d110c31..c7cf6f265 100644 --- a/get-shit-done/workflows/quick.md +++ b/get-shit-done/workflows/quick.md @@ -192,7 +192,8 @@ DEFAULT_BRANCH=$(git symbolic-ref --quiet --short refs/remotes/origin/HEAD 2>/de DEFAULT_BRANCH=${DEFAULT_BRANCH:-main} if git show-ref --verify --quiet "refs/heads/$branch_name"; then - git switch "$branch_name" + git switch "$branch_name" \ + || { echo "ERROR: Could not switch to existing quick-task branch '$branch_name'." >&2; exit 1; } else # Fetch the default branch so origin/$DEFAULT_BRANCH is current. If the fetch # fails (offline, no remote, auth failure) AND we have no local copy of @@ -219,12 +220,14 @@ else # Always pin the new branch to origin/$DEFAULT_BRANCH so the start point is # deterministic regardless of which branch we are currently on (#2916). - git checkout -b "$branch_name" "origin/$DEFAULT_BRANCH" -fi + git checkout -b "$branch_name" "origin/$DEFAULT_BRANCH" \ + || { echo "ERROR: Could not create '$branch_name' from origin/$DEFAULT_BRANCH (#2916)." >&2; exit 1; } -# Warn only when HEAD did NOT fork from origin/$DEFAULT_BRANCH (merge-base ≠ tip) — #2916. -if MB=$(git merge-base HEAD "origin/${DEFAULT_BRANCH}" 2>/dev/null) && DT=$(git rev-parse --verify --quiet "refs/remotes/origin/${DEFAULT_BRANCH}" 2>/dev/null) && [ "$MB" != "$DT" ]; then - echo "WARNING: Quick-task branch '$branch_name' does not fork from origin/${DEFAULT_BRANCH}; verify the base is intentional before continuing." + # Warn only on fresh creation when HEAD did NOT fork from origin/$DEFAULT_BRANCH — #2916. + # Skipped on resume because origin may have advanced legitimately since first creation. + if MB=$(git merge-base HEAD "origin/${DEFAULT_BRANCH}" 2>/dev/null) && DT=$(git rev-parse --verify --quiet "refs/remotes/origin/${DEFAULT_BRANCH}" 2>/dev/null) && [ "$MB" != "$DT" ]; then + echo "WARNING: Quick-task branch '$branch_name' does not fork from origin/${DEFAULT_BRANCH}; verify the base is intentional before continuing." + fi fi ```