From 48ec6cde83efde56a27ac808fd8782c8fbb6c5e5 Mon Sep 17 00:00:00 2001
From: dan <dan@delpad>
Date: Thu, 15 Jan 2026 09:37:37 -0800
Subject: [PATCH] fix(worker): address code review findings

- Fix rollback to handle partial branch creation (skills-yylq)
  - Pre-compute branch/worktree names before createWorktree
  - Use gitBranchExists() and dirExists() for robust cleanup
- Add step context to error messages (skills-ux6h)
  - Track currentStep through spawn process
  - Error now shows which step failed
- Deduplicate success output block (skills-qjln)
  - Consolidated to single block with conditional review line
- Simplify use-skills.sh auth symlink (skills-475o)
  - One-liner with || instead of nested if
- Fix inconsistent default branch in git.nim (skills-fext)
  - Changed default from "origin/integration" to "main"

Closes skills-yylq, skills-ux6h, skills-qjln, skills-475o, skills-fext

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 bin/use-skills.sh  |  6 ++----
 src/worker.nim     | 37 ++++++++++++++++++++++---------------
 src/worker/git.nim |  7 ++++++-
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/bin/use-skills.sh b/bin/use-skills.sh
index 6267215..db72915 100755
--- a/bin/use-skills.sh
+++ b/bin/use-skills.sh
@@ -19,10 +19,8 @@ export CODEX_HOME="${CODEX_HOME:-$PWD/.codex}"
 # Ensure global auth is available in per-repo CODEX_HOME to prevent login resets
 if [[ -n "${CODEX_HOME:-}" && -f "$HOME/.codex/auth.json" ]]; then
   mkdir -p "$CODEX_HOME"
-  # Only link if not already linked/present to avoid constant touching
-  if [[ ! -e "$CODEX_HOME/auth.json" ]]; then
-    ln -sf "$HOME/.codex/auth.json" "$CODEX_HOME/auth.json"
-  fi
+  # Check before linking to avoid unnecessary unlink+link on every direnv trigger
+  [[ -e "$CODEX_HOME/auth.json" ]] || ln -sf "$HOME/.codex/auth.json" "$CODEX_HOME/auth.json"
 fi
 
 # Default repo - uses local git, override with SKILLS_REPO for remote
diff --git a/src/worker.nim b/src/worker.nim
index 32c1840..eeb10e0 100644
--- a/src/worker.nim
+++ b/src/worker.nim
@@ -33,38 +33,45 @@ proc spawn(taskId: string, description: string = "",
     echo "  Worktree: ", existing.get.worktree
     quit(ExitSuccess)
 
-  var branch, worktree: string
+  # Pre-compute names so rollback works even if createWorktree fails mid-way
+  let branch = branchName(taskId)
+  let worktree = worktreePath(taskId)
+  var branchCreated, worktreeCreated = false
+  var currentStep = "initializing"
+
   try:
     # Create git worktree
-    (branch, worktree) = createWorktree(taskId, fromBranch, noFetch)
+    currentStep = "creating git worktree"
+    discard createWorktree(taskId, fromBranch, noFetch)
+    branchCreated = true
+    worktreeCreated = true
 
     # Create context file
+    currentStep = "creating context file"
     discard createWorkerContext(taskId, branch, worktree, description)
 
     # Create worker in DB
+    currentStep = "registering in database"
     discard db.createWorker(taskId, branch, worktree, description)
 
     # Enable review-gate for this task
-    if enableReview(taskId):
-      echo "Created worker: ", taskId
-      echo "  Branch: ", branch
-      echo "  Worktree: ", worktree
-      echo "  State: ASSIGNED"
+    let reviewEnabled = enableReview(taskId)
+    echo "Created worker: ", taskId
+    echo "  Branch: ", branch
+    echo "  Worktree: ", worktree
+    echo "  State: ASSIGNED"
+    if reviewEnabled:
       echo "  Review: enabled"
     else:
-      echo "Created worker: ", taskId
-      echo "  Branch: ", branch
-      echo "  Worktree: ", worktree
-      echo "  State: ASSIGNED"
       echo "  Review: (review-gate not available - install 'review-gate' for review integration)"
 
   except CatchableError as e:
-    echo "Error spawning worker: ", e.msg
-    # Rollback
-    if worktree != "":
+    echo "Error spawning worker (", currentStep, "): ", e.msg
+    # Rollback in reverse order
+    if worktreeCreated or dirExists(worktree):
       echo "Rolling back worktree..."
       removeWorktree(taskId)
-    if branch != "":
+    if branchCreated or gitBranchExists(branch):
       echo "Rolling back branch..."
       removeBranch(taskId, remote = false)
     quit(1)
diff --git a/src/worker/git.nim b/src/worker/git.nim
index 837bd36..a9cbed9 100644
--- a/src/worker/git.nim
+++ b/src/worker/git.nim
@@ -33,7 +33,12 @@ proc runGitCheck*(args: openArray[string], workDir = ""): string =
     raise newException(GitError, &"Git command failed ({code}): {output}")
   return output.strip()
 
-proc createWorktree*(taskId: string, fromBranch: string = "origin/integration", noFetch: bool = false): tuple[branch, worktree: string] =
+proc gitBranchExists*(branch: string): bool =
+  ## Check if a local branch exists
+  let (_, code) = runGit(["rev-parse", "--verify", branch])
+  return code == 0
+
+proc createWorktree*(taskId: string, fromBranch: string = "main", noFetch: bool = false): tuple[branch, worktree: string] =
   ## Create a worktree for a task
   let branch = branchName(taskId)
   let worktree = worktreePath(taskId)