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>
This commit is contained in:
parent
9f096dea0d
commit
48ec6cde83
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue