loop: always use branch sketch-wip in innie
Modify innie sketch to always create and work on a dedicated 'sketch-wip'
branch for all git operations, pushing this branch to outie instead of
pushing HEAD directly.
Having a dedicated branch name makes it clearer how to operate
inside the container, for both humans and sketch.
It also prevents the container from pushing whatever transient
commits occur while sketch does (say) a bisection or other git work.
It should also prevent sketch from constantly spinning up new
branches as it starts new tasks in a long conversation.
I'd rather have called the branch 'sketch' instead of 'sketch-wip',
but that conflicts with all the branches called 'sketch/foo'. Alas.
This was mostly written by Josh, but I made it work whether or not
sketch-wip already exists as a branch.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s4ea6db2873a60129k
diff --git a/loop/agent.go b/loop/agent.go
index 1d21a48..88e55da 100644
--- a/loop/agent.go
+++ b/loop/agent.go
@@ -316,11 +316,11 @@
}
// AgentGitState holds the state necessary for pushing to a remote git repo
-// when HEAD changes. If gitRemoteAddr is set, then we push to sketch/
+// when sketch branch changes. If gitRemoteAddr is set, then we push to sketch/
// any time we notice we need to.
type AgentGitState struct {
mu sync.Mutex // protects following
- lastHEAD string // hash of the last HEAD that was pushed to the host
+ lastSketch string // hash of the last sketch branch that was pushed to the host
gitRemoteAddr string // HTTP URL of the host git repo
upstream string // upstream branch for git work
seenCommits map[string]bool // Track git commits we've already seen (by hash)
@@ -1081,7 +1081,8 @@
if out, err := cmd.CombinedOutput(); err != nil {
return fmt.Errorf("git fetch: %s: %w", out, err)
}
- cmd = exec.CommandContext(ctx, "git", "checkout", "-f", a.config.Commit)
+ // The -B resets the branch if it already exists (or creates it if it doesn't)
+ cmd = exec.CommandContext(ctx, "git", "checkout", "-f", "-B", "sketch-wip", a.config.Commit)
cmd.Dir = a.workingDir
if checkoutOut, err := cmd.CombinedOutput(); err != nil {
// Remove git hooks if they exist and retry
@@ -1102,7 +1103,7 @@
return fmt.Errorf("git checkout %s failed even after removing hooks: %s: %w", a.config.Commit, retryOut, retryErr)
}
} else {
- return fmt.Errorf("git checkout %s: %s: %w", a.config.Commit, checkoutOut, err)
+ return fmt.Errorf("git checkout -f -B sketch-wip %s: %s: %w", a.config.Commit, checkoutOut, err)
}
}
}
@@ -1148,7 +1149,7 @@
a.codereview = codereview
}
- a.gitState.lastHEAD = a.SketchGitBase()
+ a.gitState.lastSketch = a.SketchGitBase()
a.convo = a.initConvo()
close(a.ready)
return nil
@@ -1859,15 +1860,15 @@
return msgs, nil, nil
}
- head, err := resolveRef(ctx, repoRoot, "HEAD")
+ sketch, err := resolveRef(ctx, repoRoot, "sketch-wip")
if err != nil {
return msgs, nil, err
}
- if head == ags.lastHEAD {
+ if sketch == ags.lastSketch {
return msgs, nil, nil // nothing to do
}
defer func() {
- ags.lastHEAD = head
+ ags.lastSketch = sketch
}()
// Get new commits. Because it's possible that the agent does rebases, fixups, and
@@ -1879,7 +1880,7 @@
// Format: <hash>\0<subject>\0<body>\0
// This uses NULL bytes as separators to avoid issues with newlines in commit messages
// Limit to 100 commits to avoid overwhelming the user
- cmd := exec.CommandContext(ctx, "git", "log", "-n", "100", "--pretty=format:%H%x00%s%x00%b%x00", "^"+baseRef, head)
+ cmd := exec.CommandContext(ctx, "git", "log", "-n", "100", "--pretty=format:%H%x00%s%x00%b%x00", "^"+baseRef, sketch)
cmd.Dir = repoRoot
output, err := cmd.Output()
if err != nil {
@@ -1889,16 +1890,16 @@
// Parse git log output and filter out already seen commits
parsedCommits := parseGitLog(string(output))
- var headCommit *GitCommit
+ var sketchCommit *GitCommit
// Filter out commits we've already seen
for _, commit := range parsedCommits {
- if commit.Hash == head {
- headCommit = &commit
+ if commit.Hash == sketch {
+ sketchCommit = &commit
}
- // Skip if we've seen this commit before. If our head has changed, always include that.
- if ags.seenCommits[commit.Hash] && commit.Hash != head {
+ // Skip if we've seen this commit before. If our sketch branch has changed, always include that.
+ if ags.seenCommits[commit.Hash] && commit.Hash != sketch {
continue
}
@@ -1910,12 +1911,12 @@
}
if ags.gitRemoteAddr != "" {
- if headCommit == nil {
+ if sketchCommit == nil {
// I think this can only happen if we have a bug or if there's a race.
- headCommit = &GitCommit{}
- headCommit.Hash = head
- headCommit.Subject = "unknown"
- commits = append(commits, headCommit)
+ sketchCommit = &GitCommit{}
+ sketchCommit.Hash = sketch
+ sketchCommit.Subject = "unknown"
+ commits = append(commits, sketchCommit)
}
// TODO: I don't love the force push here. We could see if the push is a fast-forward, and,
@@ -1933,7 +1934,7 @@
}
branch := ags.branchNameLocked(branchPrefix)
- cmd = exec.Command("git", "push", "--force", ags.gitRemoteAddr, "HEAD:refs/heads/"+branch)
+ cmd = exec.Command("git", "push", "--force", ags.gitRemoteAddr, "sketch-wip:refs/heads/"+branch)
cmd.Dir = repoRoot
out, err = cmd.CombinedOutput()
@@ -1953,7 +1954,7 @@
msgs = append(msgs, errorMessage(fmt.Errorf("git push to host: %s: %v", out, err)))
} else {
finalBranch := ags.branchNameLocked(branchPrefix)
- headCommit.PushedBranch = finalBranch
+ sketchCommit.PushedBranch = finalBranch
if ags.retryNumber != originalRetryNumber {
// Notify user that the branch name was changed, and why
msgs = append(msgs, AgentMessage{