From 8c03fa4589a9f86126bbde653c8118e967edb722 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Sun, 1 Mar 2026 06:05:50 +0900 Subject: [PATCH 1/2] feat: auto-merge heartbeat worktree branch into main on completion Heartbeat worktrees were auto-committed but never merged back, so their changes (e.g. project scaffolding) were effectively lost. Now the heartbeat defer attempts a fast-forward merge into the base branch after auto-commit. On conflict, the merge is aborted and the user is notified to merge manually. Co-Authored-By: Claude Opus 4.6 --- pkg/agent/loop.go | 52 ++++++++++++++++++++------- pkg/git/worktree.go | 28 +++++++++++++++ pkg/git/worktree_test.go | 78 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 12 deletions(-) diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 218ef14c5..0d8f71bb1 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -25,6 +25,7 @@ import ( "github.com/sipeed/picoclaw/pkg/channels" "github.com/sipeed/picoclaw/pkg/config" "github.com/sipeed/picoclaw/pkg/constants" + "github.com/sipeed/picoclaw/pkg/git" "github.com/sipeed/picoclaw/pkg/logger" "github.com/sipeed/picoclaw/pkg/media" "github.com/sipeed/picoclaw/pkg/orch" @@ -944,24 +945,51 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt // Guarantee heartbeat worktree cleanup on ALL exit paths (error, panic, normal). // Wait for spawned subagents first so they aren't killed mid-flight. + // After auto-commit, attempt to merge the worktree branch into main. defer func() { if opts.Background { if agent.SubagentMgr != nil { agent.SubagentMgr.WaitAll(35 * time.Minute) // slightly above spawnTimeout } - if agent.IsInWorktree(opts.SessionKey) { - commitMsg := "heartbeat: auto-save" - wtResult, _ := agent.DeactivateWorktree(opts.SessionKey, commitMsg, false) - if wtResult != nil && wtResult.CommitsAhead > 0 && !constants.IsInternalChannel(opts.Channel) { - cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 5*time.Second) - _ = al.bus.PublishOutbound(cleanupCtx, bus.OutboundMessage{ - Channel: opts.Channel, - ChatID: opts.ChatID, - Content: fmt.Sprintf("Heartbeat made code changes on branch `%s` (%d commits).", - wtResult.Branch, wtResult.CommitsAhead), - }) - cleanupCancel() + wt := agent.GetWorktree(opts.SessionKey) + if wt != nil { + // 1. Auto-commit uncommitted changes in worktree + if git.HasUncommittedChanges(wt.Path) { + _ = git.AutoCommit(wt.Path, "heartbeat: auto-save") } + + // 2. Check if there are unique commits worth merging + repoRoot := git.FindRepoRoot(agent.Workspace) + ahead := git.CommitsAhead(repoRoot, wt.BaseBranch, wt.Branch) + + if ahead > 0 && repoRoot != "" { + // 3. Try fast-forward merge into base branch + mr := git.MergeWorktreeBranch(repoRoot, wt) + + // 4. Notify based on merge result + if !constants.IsInternalChannel(opts.Channel) { + cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 5*time.Second) + if mr.Merged { + _ = al.bus.PublishOutbound(cleanupCtx, bus.OutboundMessage{ + Channel: opts.Channel, + ChatID: opts.ChatID, + Content: fmt.Sprintf("Heartbeat: merged %d commit(s) to %s.", + ahead, wt.BaseBranch), + }) + } else if mr.Conflict { + _ = al.bus.PublishOutbound(cleanupCtx, bus.OutboundMessage{ + Channel: opts.Channel, + ChatID: opts.ChatID, + Content: fmt.Sprintf("Heartbeat: merge conflict on branch `%s` — manual merge needed.", + mr.Branch), + }) + } + cleanupCancel() + } + } + + // 5. Dispose worktree (branch auto-deleted if merged, kept if conflict) + agent.DeactivateWorktree(opts.SessionKey, "", false) } } }() diff --git a/pkg/git/worktree.go b/pkg/git/worktree.go index 1923f55d9..d733d1428 100644 --- a/pkg/git/worktree.go +++ b/pkg/git/worktree.go @@ -193,6 +193,34 @@ func SafeDispose(repoDir string, wt *WorktreeInfo) DisposeResult { return result } +// MergeResult describes the outcome of a worktree branch merge attempt. +type MergeResult struct { + Merged bool // true if merge succeeded + Branch string // branch name that was merged + Conflict bool // true if merge failed due to conflict +} + +// MergeWorktreeBranch attempts to merge the worktree branch into the base branch. +// On conflict, it aborts the merge and returns Conflict=true. +// Must be called AFTER auto-commit and BEFORE SafeDispose. +func MergeWorktreeBranch(repoDir string, wt *WorktreeInfo) MergeResult { + result := MergeResult{Branch: wt.Branch} + + mergeCmd := exec.Command("git", "merge", "--no-edit", wt.Branch) + mergeCmd.Dir = repoDir + if err := mergeCmd.Run(); err != nil { + // Merge failed — abort and report conflict + abortCmd := exec.Command("git", "merge", "--abort") + abortCmd.Dir = repoDir + abortCmd.Run() // best-effort + result.Conflict = true + return result + } + + result.Merged = true + return result +} + // PruneOrphaned runs git worktree prune and removes dirs in worktreesDir // that aren't valid git worktrees. func PruneOrphaned(repoDir, worktreesDir string) { diff --git a/pkg/git/worktree_test.go b/pkg/git/worktree_test.go index 41d2d97aa..0e24aba20 100644 --- a/pkg/git/worktree_test.go +++ b/pkg/git/worktree_test.go @@ -201,6 +201,84 @@ func TestCommitsAhead(t *testing.T) { } } +func TestMergeWorktreeBranch_Success(t *testing.T) { + dir := initTestRepo(t) + baseBranch := CurrentBranch(dir) + wtPath := filepath.Join(dir, ".picoclaw", "worktrees", "merge-ok") + + wt, err := CreateWorktree(dir, wtPath, "plan/merge-ok") + if err != nil { + t.Fatalf("CreateWorktree: %v", err) + } + + // Make a change in the worktree and commit + os.WriteFile(filepath.Join(wtPath, "merged-file.txt"), []byte("hello from worktree"), 0o644) + if err := AutoCommit(wtPath, "add merged-file"); err != nil { + t.Fatalf("AutoCommit: %v", err) + } + + // Merge into base branch + mr := MergeWorktreeBranch(dir, wt) + if !mr.Merged { + t.Fatal("expected Merged=true") + } + if mr.Conflict { + t.Fatal("expected Conflict=false") + } + if mr.Branch != "plan/merge-ok" { + t.Errorf("Branch = %q, want %q", mr.Branch, "plan/merge-ok") + } + + // Verify the file exists on the base branch + checkoutCmd := exec.Command("git", "checkout", baseBranch) + checkoutCmd.Dir = dir + checkoutCmd.Run() + + if _, err := os.Stat(filepath.Join(dir, "merged-file.txt")); os.IsNotExist(err) { + t.Fatal("merged-file.txt should exist on base branch after merge") + } +} + +func TestMergeWorktreeBranch_Conflict(t *testing.T) { + dir := initTestRepo(t) + wtPath := filepath.Join(dir, ".picoclaw", "worktrees", "merge-conflict") + + wt, err := CreateWorktree(dir, wtPath, "plan/merge-conflict") + if err != nil { + t.Fatalf("CreateWorktree: %v", err) + } + + // Make a change on the base branch + os.WriteFile(filepath.Join(dir, "conflict.txt"), []byte("base content"), 0o644) + if err := AutoCommit(dir, "add conflict.txt on base"); err != nil { + t.Fatalf("AutoCommit base: %v", err) + } + + // Make a conflicting change in the worktree + os.WriteFile(filepath.Join(wtPath, "conflict.txt"), []byte("worktree content"), 0o644) + if err := AutoCommit(wtPath, "add conflict.txt on worktree"); err != nil { + t.Fatalf("AutoCommit worktree: %v", err) + } + + // Attempt merge — should conflict + mr := MergeWorktreeBranch(dir, wt) + if mr.Merged { + t.Fatal("expected Merged=false on conflict") + } + if !mr.Conflict { + t.Fatal("expected Conflict=true") + } + + // Verify base branch file is unchanged (merge was aborted) + content, err := os.ReadFile(filepath.Join(dir, "conflict.txt")) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if string(content) != "base content" { + t.Errorf("conflict.txt = %q, want %q (merge should have been aborted)", string(content), "base content") + } +} + func TestPruneOrphaned(t *testing.T) { dir := initTestRepo(t) From fa4ac0f5365ebdad1b60b79e8fdf0481b83be760 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Sun, 1 Mar 2026 06:11:12 +0900 Subject: [PATCH 2/2] fix: resolve govet shadow warnings in merge conflict test Rename err variables to avoid shadowing the outer CreateWorktree error. Co-Authored-By: Claude Opus 4.6 --- pkg/git/worktree_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/git/worktree_test.go b/pkg/git/worktree_test.go index 0e24aba20..a80db5376 100644 --- a/pkg/git/worktree_test.go +++ b/pkg/git/worktree_test.go @@ -243,9 +243,9 @@ func TestMergeWorktreeBranch_Conflict(t *testing.T) { dir := initTestRepo(t) wtPath := filepath.Join(dir, ".picoclaw", "worktrees", "merge-conflict") - wt, err := CreateWorktree(dir, wtPath, "plan/merge-conflict") - if err != nil { - t.Fatalf("CreateWorktree: %v", err) + wt, createErr := CreateWorktree(dir, wtPath, "plan/merge-conflict") + if createErr != nil { + t.Fatalf("CreateWorktree: %v", createErr) } // Make a change on the base branch @@ -270,9 +270,9 @@ func TestMergeWorktreeBranch_Conflict(t *testing.T) { } // Verify base branch file is unchanged (merge was aborted) - content, err := os.ReadFile(filepath.Join(dir, "conflict.txt")) - if err != nil { - t.Fatalf("ReadFile: %v", err) + content, readErr := os.ReadFile(filepath.Join(dir, "conflict.txt")) + if readErr != nil { + t.Fatalf("ReadFile: %v", readErr) } if string(content) != "base content" { t.Errorf("conflict.txt = %q, want %q (merge should have been aborted)", string(content), "base content")