From 3c4cf6af6f9c1860fe9feb98da9e0628c689cc85 Mon Sep 17 00:00:00 2001 From: dj-oyu <68707227+dj-oyu@users.noreply.github.com> Date: Tue, 24 Feb 2026 12:35:23 +0900 Subject: [PATCH] fix: move heartbeat worktree cleanup to defer for guaranteed execution The heartbeat worktree cleanup was inline after runLLMIteration, so errors or panics from the LLM loop would skip cleanup entirely, leaving orphaned worktrees with uncommitted changes. Move to a defer block registered early in runAgentLoop to guarantee execution on all exit paths. Also document the missing human intervention mechanism for heartbeat worktrees as a known gap in CLAUDE.md. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 1 + pkg/agent/loop.go | 30 ++++++++++++++++-------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1213d74a5..985edfac1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,3 +26,4 @@ Lint: `golangci-lint run` ## Known Gaps - **Mini App log viewer has no frontend tests**: `renderLogs()` in `pkg/miniapp/static/index.html` is inline vanilla JS with no unit/E2E test coverage. Backend (Go) tests cover `RecentLogs`, `SanitizeFields`, and JSON serialization, but nothing verifies the JS rendering. This allowed the Fields display bug (fields sent but not rendered) to ship undetected. +- **No human intervention for heartbeat worktrees**: Heartbeat sessions create git worktrees (`.picoclaw/worktrees/heartbeat-YYYYMMDD/`) but there is no CLI or Mini App command to list, inspect, or manually dispose them. Need a `/plan worktrees` command (or similar) that shows active worktrees with branch/commit info and allows manual merge/dispose. `PruneOrphaned` on startup only removes directories without auto-committing first, so uncommitted changes in orphaned worktrees are silently lost. diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 93919d9de..704f90b37 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -708,6 +708,22 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt interrupt: make(chan string, 1), } + // Guarantee heartbeat worktree cleanup on ALL exit paths (error, panic, normal). + defer func() { + if opts.Background && 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) { + al.bus.PublishOutbound(bus.OutboundMessage{ + Channel: opts.Channel, + ChatID: opts.ChatID, + Content: fmt.Sprintf("Heartbeat made code changes on branch `%s` (%d commits).", + wtResult.Branch, wtResult.CommitsAhead), + }) + } + } + }() + // For background tasks (cron/heartbeat), generate a TaskID and send notification isBackgroundTask := opts.Background && al.state != nil if isBackgroundTask && opts.TaskID == "" { @@ -1003,20 +1019,6 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt "final_length": len(finalContent), }) - // 10. Heartbeat worktree cleanup: auto-commit and dispose after background task - if opts.Background && 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) { - al.bus.PublishOutbound(bus.OutboundMessage{ - Channel: opts.Channel, - ChatID: opts.ChatID, - Content: fmt.Sprintf("Heartbeat made code changes on branch `%s` (%d commits).", - wtResult.Branch, wtResult.CommitsAhead), - }) - } - } - return finalContent, nil }