fix: varargs binding bug in git.nim runGit/runGitCheck
Nim's varargs doesn't consume the last positional argument when there's
a trailing parameter with a default value. This caused calls like
`runGit("fetch", "origin")` to be parsed as:
- args = ["fetch"]
- workDir = "origin"
Instead of the intended:
- args = ["fetch", "origin"]
- workDir = ""
Fixed by changing from varargs to openArray, which requires explicit
array syntax at call sites: `runGit(["fetch", "origin"])`.
Also includes P2 bug fixes:
- Start heartbeat before state transition (skills-qekj)
- Reject symlinks when reading context file (skills-16zf)
- Case-insensitive conflict detection (skills-n3qp)
Smoke tested: spawn, status, start, show, cancel all work.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
dan
parent
3afd621bb4
commit
405e6b9aee
|
|
@ -7,8 +7,9 @@ import std/[os, osproc, strutils, strformat, streams]
|
|||
import ./types
|
||||
import ./utils
|
||||
|
||||
proc runGit*(args: varargs[string], workDir: string = ""): tuple[output: string, exitCode: int] =
|
||||
proc runGit*(args: openArray[string], workDir = ""): tuple[output: string, exitCode: int] =
|
||||
## Run a git command and return output + exit code
|
||||
## Note: uses openArray instead of varargs to avoid binding issues
|
||||
var cmd = "git"
|
||||
var fullArgs: seq[string] = @[]
|
||||
|
||||
|
|
@ -25,7 +26,7 @@ proc runGit*(args: varargs[string], workDir: string = ""): tuple[output: string,
|
|||
result.exitCode = process.waitForExit()
|
||||
process.close()
|
||||
|
||||
proc runGitCheck*(args: varargs[string], workDir: string = ""): string =
|
||||
proc runGitCheck*(args: openArray[string], workDir = ""): string =
|
||||
## Run git command, raise GitError on failure
|
||||
let (output, code) = runGit(args, workDir)
|
||||
if code != 0:
|
||||
|
|
@ -38,16 +39,16 @@ proc createWorktree*(taskId: string, fromBranch: string = "origin/integration"):
|
|||
let worktree = worktreePath(taskId)
|
||||
|
||||
# Fetch latest (warn but continue on failure - may be offline)
|
||||
let (fetchOut, fetchCode) = runGit("fetch", "origin")
|
||||
let (fetchOut, fetchCode) = runGit(["fetch", "origin"])
|
||||
if fetchCode != 0:
|
||||
logWarn("createWorktree", "fetch failed, continuing with local state: " & fetchOut.strip())
|
||||
|
||||
# Create branch from base
|
||||
discard runGitCheck("branch", branch, fromBranch)
|
||||
discard runGitCheck(["branch", branch, fromBranch])
|
||||
|
||||
# Create worktree
|
||||
createDir(parentDir(worktree))
|
||||
discard runGitCheck("worktree", "add", worktree, branch)
|
||||
discard runGitCheck(["worktree", "add", worktree, branch])
|
||||
|
||||
return (branch, worktree)
|
||||
|
||||
|
|
@ -55,33 +56,33 @@ proc removeWorktree*(taskId: string) =
|
|||
## Remove a worktree
|
||||
let worktree = worktreePath(taskId)
|
||||
if dirExists(worktree):
|
||||
let (output, code) = runGit("worktree", "remove", "--force", worktree)
|
||||
let (output, code) = runGit(["worktree", "remove", "--force", worktree])
|
||||
if code != 0:
|
||||
logWarn("removeWorktree", "failed for " & taskId & ": " & output.strip())
|
||||
|
||||
proc removeBranch*(taskId: string, remote: bool = true) =
|
||||
## Remove feature branch
|
||||
let branch = branchName(taskId)
|
||||
let (localOut, localCode) = runGit("branch", "-D", branch)
|
||||
let (localOut, localCode) = runGit(["branch", "-D", branch])
|
||||
if localCode != 0:
|
||||
logWarn("removeBranch", "local delete failed for " & branch & ": " & localOut.strip())
|
||||
if remote:
|
||||
let (remoteOut, remoteCode) = runGit("push", "origin", "--delete", branch)
|
||||
let (remoteOut, remoteCode) = runGit(["push", "origin", "--delete", branch])
|
||||
if remoteCode != 0:
|
||||
logWarn("removeBranch", "remote delete failed for " & branch & ": " & remoteOut.strip())
|
||||
|
||||
proc rebaseOnIntegration*(worktree: string): bool =
|
||||
## Rebase worktree on integration branch. Returns false on conflict.
|
||||
let (fetchOut, fetchCode) = runGit("fetch", "origin", workDir = worktree)
|
||||
let (fetchOut, fetchCode) = runGit(["fetch", "origin"], workDir = worktree)
|
||||
if fetchCode != 0:
|
||||
logWarn("rebaseOnIntegration", "fetch failed, using local state: " & fetchOut.strip())
|
||||
let (output, code) = runGit("rebase", "origin/integration", workDir = worktree)
|
||||
let (output, code) = runGit(["rebase", "origin/integration"], workDir = worktree)
|
||||
|
||||
if code != 0:
|
||||
if "conflict" in output.toLowerAscii():
|
||||
return false
|
||||
# Other error - abort and raise
|
||||
discard runGit("rebase", "--abort", workDir = worktree)
|
||||
discard runGit(["rebase", "--abort"], workDir = worktree)
|
||||
raise newException(GitError, &"Rebase failed: {output}")
|
||||
|
||||
return true
|
||||
|
|
@ -93,7 +94,7 @@ proc isRebaseInProgress*(worktree: string): bool =
|
|||
|
||||
proc pushBranch*(worktree: string, branch: string) =
|
||||
## Push branch to origin
|
||||
discard runGitCheck("push", "-u", "origin", branch, workDir = worktree)
|
||||
discard runGitCheck(["push", "-u", "origin", branch], workDir = worktree)
|
||||
|
||||
proc mergeToIntegration*(taskId: string, maxRetries: int = 3): bool =
|
||||
## Merge feature branch to integration with retry loop
|
||||
|
|
@ -101,23 +102,23 @@ proc mergeToIntegration*(taskId: string, maxRetries: int = 3): bool =
|
|||
|
||||
for attempt in 1..maxRetries:
|
||||
# Fetch latest
|
||||
discard runGitCheck("fetch", "origin", "integration", branch)
|
||||
discard runGitCheck(["fetch", "origin", "integration", branch])
|
||||
|
||||
# Checkout and reset integration
|
||||
discard runGitCheck("checkout", "integration")
|
||||
discard runGitCheck("reset", "--hard", "origin/integration")
|
||||
discard runGitCheck(["checkout", "integration"])
|
||||
discard runGitCheck(["reset", "--hard", "origin/integration"])
|
||||
|
||||
# Merge
|
||||
let (mergeOutput, mergeCode) = runGit("merge", "--no-ff", branch,
|
||||
"-m", &"Merge {branch}")
|
||||
let (mergeOutput, mergeCode) = runGit(["merge", "--no-ff", branch,
|
||||
"-m", &"Merge {branch}"])
|
||||
if mergeCode != 0:
|
||||
if "conflict" in mergeOutput.toLowerAscii():
|
||||
discard runGit("merge", "--abort")
|
||||
discard runGit(["merge", "--abort"])
|
||||
return false
|
||||
raise newException(GitError, &"Merge failed: {mergeOutput}")
|
||||
|
||||
# Push
|
||||
let (pushOutput, pushCode) = runGit("push", "origin", "integration")
|
||||
let (pushOutput, pushCode) = runGit(["push", "origin", "integration"])
|
||||
if pushCode == 0:
|
||||
return true
|
||||
|
||||
|
|
@ -132,16 +133,15 @@ proc mergeToIntegration*(taskId: string, maxRetries: int = 3): bool =
|
|||
|
||||
proc getConflictedFiles*(worktree: string): seq[string] =
|
||||
## Get list of files with conflicts
|
||||
let (output, _) = runGit("diff", "--name-only", "--diff-filter=U",
|
||||
workDir = worktree)
|
||||
let (output, _) = runGit(["diff", "--name-only", "--diff-filter=U"], workDir = worktree)
|
||||
for line in output.splitLines():
|
||||
if line.strip() != "":
|
||||
result.add(line.strip())
|
||||
|
||||
proc getBranchStatus*(worktree: string): tuple[ahead, behind: int] =
|
||||
## Get commits ahead/behind integration
|
||||
let (output, code) = runGit("rev-list", "--left-right", "--count",
|
||||
"origin/integration...HEAD", workDir = worktree)
|
||||
let (output, code) = runGit(["rev-list", "--left-right", "--count",
|
||||
"origin/integration...HEAD"], workDir = worktree)
|
||||
if code != 0:
|
||||
return (0, 0)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue