From b8fa37965828988b7ac0ddf3f585e56d7bc24a3c Mon Sep 17 00:00:00 2001 From: moltenhub-bot Date: Fri, 10 Apr 2026 22:31:02 -0700 Subject: [PATCH] moltenhub-preserve-local-task-logs-for-follow-up-runs --- cmd/harness/main.go | 105 +++++++++++++++++++++++++++- cmd/harness/main_additional_test.go | 50 ++++++++++++- cmd/harness/main_test.go | 2 +- 3 files changed, 153 insertions(+), 4 deletions(-) diff --git a/cmd/harness/main.go b/cmd/harness/main.go index 59cb6b02..25663380 100644 --- a/cmd/harness/main.go +++ b/cmd/harness/main.go @@ -38,6 +38,7 @@ const hubPingRemoteContinueDetail = "Hub endpoint ping precheck failed; continui const hubPingHeadlessNoopDetail = "Hub endpoint ping precheck failed with UI disabled and no Hub credentials configured; startup completed without remote transport." const gitHubCLIPackageLabel = "github-cli (gh)" const gitHubCLIAuthRecommendation = "Run `gh auth login` (the GitHub CLI binary from the `github-cli` package) or set GH_TOKEN before dispatching tasks." +const followUpTaskLogArchiveSubdir = "followup" const hubBootDiagnosticTimeout = 10 * time.Second const hubPingDiagnosticTimeout = 5 * time.Second @@ -912,7 +913,7 @@ func failureFollowUpRunConfig( failedRunCfg config.Config, logRoot string, ) config.Config { - logPaths := failurefollowup.TaskLogPaths(logRoot, failedRequestID) + logPaths := followUpTaskLogPaths(logRoot, failedRequestID) baseBranch, targetSubdir := failurefollowup.FollowUpTargeting( failedRunCfg.BaseBranch, failedRunCfg.TargetSubdir, @@ -933,7 +934,7 @@ func unexpectedNoChangesFollowUpRunConfig( logRoot string, ) config.Config { runCfg.ApplyDefaults() - logPaths := existingPaths(failurefollowup.TaskLogPaths(logRoot, requestID)) + logPaths := existingPaths(followUpTaskLogPaths(logRoot, requestID)) baseBranch := strings.TrimSpace(runCfg.BaseBranch) return config.Config{ Repos: unexpectedNoChangesFollowUpRepos(runCfg), @@ -1124,6 +1125,106 @@ func existingPaths(paths []string) []string { return existing } +func followUpTaskLogPaths(logRoot, requestID string) []string { + paths := taskLogPaths(logRoot, requestID) + if _, ok := localTaskLogDir(logRoot, requestID); !ok { + return paths + } + + archived := archiveLocalTaskLogsForFollowUp(logRoot, requestID) + if len(archived) > 0 { + return archived + } + return paths +} + +func archiveLocalTaskLogsForFollowUp(logRoot, requestID string) []string { + sourceDir, ok := localTaskLogDir(logRoot, requestID) + if !ok { + return nil + } + sourceDir = strings.TrimSpace(sourceDir) + if sourceDir == "" { + return nil + } + if stat, err := os.Stat(sourceDir); err != nil || !stat.IsDir() { + return nil + } + + archiveDir, ok := followUpTaskLogArchiveDir(logRoot, requestID) + if !ok { + return nil + } + if err := os.MkdirAll(archiveDir, 0o755); err != nil { + return nil + } + + paths := []string{archiveDir} + for _, fileName := range []string{legacyTaskLogFileName, logFileName} { + sourcePath := filepath.Join(sourceDir, fileName) + archivePath := filepath.Join(archiveDir, fileName) + copied, err := copyFileIfPresent(sourcePath, archivePath) + if err != nil || !copied { + continue + } + paths = append(paths, archivePath) + } + if len(paths) == 1 { + return nil + } + return paths +} + +func copyFileIfPresent(sourcePath, targetPath string) (bool, error) { + sourcePath = strings.TrimSpace(sourcePath) + targetPath = strings.TrimSpace(targetPath) + if sourcePath == "" || targetPath == "" { + return false, nil + } + + in, err := os.Open(sourcePath) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return false, err + } + defer in.Close() + + out, err := os.OpenFile(targetPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, maxLogFileOpenMode) + if err != nil { + return false, err + } + if _, err := io.Copy(out, in); err != nil { + _ = out.Close() + return false, err + } + if err := out.Close(); err != nil { + return false, err + } + return true, nil +} + +func followUpTaskLogArchiveDir(logRoot, requestID string) (string, bool) { + logRoot = strings.TrimSpace(logRoot) + if logRoot == "" { + return "", false + } + subdir, ok := failurefollowup.IdentifierSubdir(requestID) + if !ok { + return "", false + } + subdir = filepath.Clean(subdir) + if subdir == "." || subdir == "" || subdir == ".." { + return "", false + } + if filepath.IsAbs(subdir) || strings.HasPrefix(subdir, ".."+string(filepath.Separator)) { + return "", false + } + + return filepath.Join(logRoot, followUpTaskLogArchiveSubdir, subdir), true +} + func taskLogPaths(logRoot, requestID string) []string { return failurefollowup.TaskLogPaths(logRoot, requestID) } diff --git a/cmd/harness/main_additional_test.go b/cmd/harness/main_additional_test.go index 0c6353da..8aa1bc33 100644 --- a/cmd/harness/main_additional_test.go +++ b/cmd/harness/main_additional_test.go @@ -621,9 +621,12 @@ func TestUnexpectedNoChangesFollowUpRunConfigPreservesTaskTargetingAndAddsContex t.Fatalf("Repos = %v, want %v", got, want) } + expectedLogDir := filepath.Join(logRoot, followUpTaskLogArchiveSubdir, "local", "1712345678", "000001") + for _, want := range []string{ "Review the previous local task logs first.", - filepath.Join(logRoot, "local", "1712345678", "000001"), + expectedLogDir, + filepath.Join(expectedLogDir, logFileName), "Observed no-change context:", "- request_id=local-1712345678-000001", "- workspace_dir=/tmp/run-123", @@ -725,6 +728,51 @@ func TestUnexpectedNoChangesFollowUpRunConfigUsesNoPathGuidanceWhenTaskLogsMissi } } +func TestFollowUpTaskLogPathsArchivesLocalTaskLogs(t *testing.T) { + t.Parallel() + + logRoot := filepath.Join(t.TempDir(), ".log") + requestID := "local-1712345678-000001" + sourceDir := filepath.Join(logRoot, "local", "1712345678", "000001") + if err := os.MkdirAll(sourceDir, 0o755); err != nil { + t.Fatalf("mkdir source dir: %v", err) + } + + legacyContent := []byte("legacy log\n") + if err := os.WriteFile(filepath.Join(sourceDir, legacyTaskLogFileName), legacyContent, 0o644); err != nil { + t.Fatalf("write legacy log: %v", err) + } + currentContent := []byte("current log\n") + if err := os.WriteFile(filepath.Join(sourceDir, logFileName), currentContent, 0o644); err != nil { + t.Fatalf("write current log: %v", err) + } + + paths := followUpTaskLogPaths(logRoot, requestID) + archiveDir := filepath.Join(logRoot, followUpTaskLogArchiveSubdir, "local", "1712345678", "000001") + wantPaths := []string{ + archiveDir, + filepath.Join(archiveDir, legacyTaskLogFileName), + filepath.Join(archiveDir, logFileName), + } + if !reflect.DeepEqual(paths, wantPaths) { + t.Fatalf("followUpTaskLogPaths() = %v, want %v", paths, wantPaths) + } + + if got, err := os.ReadFile(filepath.Join(archiveDir, legacyTaskLogFileName)); err != nil || string(got) != string(legacyContent) { + t.Fatalf("archived legacy log mismatch: content=%q err=%v", string(got), err) + } + if got, err := os.ReadFile(filepath.Join(archiveDir, logFileName)); err != nil || string(got) != string(currentContent) { + t.Fatalf("archived current log mismatch: content=%q err=%v", string(got), err) + } + + if err := os.RemoveAll(sourceDir); err != nil { + t.Fatalf("remove source dir: %v", err) + } + if got := existingPaths(paths); !reflect.DeepEqual(got, wantPaths) { + t.Fatalf("existingPaths(archived) = %v, want %v", got, wantPaths) + } +} + func TestShouldQueueFailureFollowUpQueuesNoDeltaFailures(t *testing.T) { t.Parallel() diff --git a/cmd/harness/main_test.go b/cmd/harness/main_test.go index cd393915..8884688e 100644 --- a/cmd/harness/main_test.go +++ b/cmd/harness/main_test.go @@ -396,7 +396,7 @@ func TestFailureFollowUpRunConfigUsesRequiredPayloadShapeAndLogContext(t *testin t.Fatalf("TargetSubdir = %q, want %q", cfg.TargetSubdir, ".") } - expectedLogDir := filepath.Join(logRoot, "local", "1712345678", "000001") + expectedLogDir := filepath.Join(logRoot, followUpTaskLogArchiveSubdir, "local", "1712345678", "000001") if !strings.Contains(cfg.Prompt, expectedLogDir) { t.Fatalf("Prompt missing log dir path %q: %q", expectedLogDir, cfg.Prompt) }