diff options
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/api/stories.go | 22 | ||||
| -rw-r--r-- | internal/api/stories_test.go | 3 | ||||
| -rw-r--r-- | internal/executor/executor.go | 2 | ||||
| -rw-r--r-- | internal/executor/executor_test.go | 32 |
4 files changed, 34 insertions, 25 deletions
diff --git a/internal/api/stories.go b/internal/api/stories.go index 3d9d25a..629ea7a 100644 --- a/internal/api/stories.go +++ b/internal/api/stories.go @@ -14,25 +14,21 @@ import ( "github.com/thepeterstone/claudomator/internal/task" ) -// createStoryBranch creates a new git branch in localPath from origin/main -// and pushes it to origin. Idempotent: treats "already exists" as success. -func createStoryBranch(localPath, branchName string) error { - // Fetch latest from origin so origin/main is up to date. - if out, err := exec.Command("git", "-C", localPath, "fetch", "origin").CombinedOutput(); err != nil { +// createStoryBranch creates a new git branch in localPath from the latest main +// and pushes it to remoteURL (the bare repo). Idempotent: treats "already exists" as success. +func createStoryBranch(localPath, branchName, remoteURL string) error { + // Fetch latest from the bare repo so we have an up-to-date base. + if out, err := exec.Command("git", "-C", localPath, "fetch", remoteURL, "main").CombinedOutput(); err != nil { return fmt.Errorf("git fetch: %w (output: %s)", err, string(out)) } - base := "origin/main" + base := "FETCH_HEAD" out, err := exec.Command("git", "-C", localPath, "checkout", "-b", branchName, base).CombinedOutput() if err != nil { if !strings.Contains(string(out), "already exists") { return fmt.Errorf("git checkout -b: %w (output: %s)", err, string(out)) } - // Branch exists; switch to it — idempotent. - if out2, err2 := exec.Command("git", "-C", localPath, "checkout", branchName).CombinedOutput(); err2 != nil { - return fmt.Errorf("git checkout: %w (output: %s)", err2, string(out2)) - } } - if out, err := exec.Command("git", "-C", localPath, "push", "origin", branchName).CombinedOutput(); err != nil { + if out, err := exec.Command("git", "-C", localPath, "push", remoteURL, branchName).CombinedOutput(); err != nil { return fmt.Errorf("git push: %w (output: %s)", err, string(out)) } return nil @@ -313,8 +309,8 @@ func (s *Server) handleApproveStory(w http.ResponseWriter, r *http.Request) { // Create the story branch (non-fatal if it fails). if input.BranchName != "" && input.ProjectID != "" { - if proj, err := s.store.GetProject(input.ProjectID); err == nil && proj.LocalPath != "" { - if err := createStoryBranch(proj.LocalPath, input.BranchName); err != nil { + if proj, err := s.store.GetProject(input.ProjectID); err == nil && proj.LocalPath != "" && proj.RemoteURL != "" { + if err := createStoryBranch(proj.LocalPath, input.BranchName, proj.RemoteURL); err != nil { s.logger.Warn("story approve: failed to create branch", "error", err, "branch", input.BranchName) } } diff --git a/internal/api/stories_test.go b/internal/api/stories_test.go index 53d15eb..342840b 100644 --- a/internal/api/stories_test.go +++ b/internal/api/stories_test.go @@ -212,7 +212,8 @@ func TestHandleStoryApprove_SetsRepositoryURL(t *testing.T) { ID: "proj-repo", Name: "claudomator", RemoteURL: "/site/git.terst.org/repos/claudomator.git", - LocalPath: "/workspace/claudomator", + // LocalPath intentionally empty: branch creation is a non-fatal side effect, + // omitting it keeps the test fast and free of real git operations. } if err := store.CreateProject(proj); err != nil { t.Fatalf("CreateProject: %v", err) diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 9c4aac1..d2b476a 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -379,7 +379,7 @@ func (p *Pool) handleRunResult(ctx context.Context, t *task.Task, exec *storage. p.consecutiveFailures[agentType]++ failures := p.consecutiveFailures[agentType] p.mu.Unlock() - if failures >= 2 { + if failures >= 3 { p.mu.Lock() p.drained[agentType] = true p.mu.Unlock() diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index 67b2764..ad496e7 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -1543,20 +1543,32 @@ func TestPool_MaxPerAgent_AllowsDifferentAgents(t *testing.T) { } } -func TestPool_ConsecutiveFailures_DrainAtTwo(t *testing.T) { +func TestPool_ConsecutiveFailures_DrainAtThree(t *testing.T) { store := testStore(t) runner := &mockRunner{err: fmt.Errorf("boom")} runners := map[string]Runner{"claude": runner} logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) - pool := NewPool(2, runners, store, logger) + pool := NewPool(3, runners, store, logger) - // Submit two failing tasks + // Two failures should NOT drain. for _, id := range []string{"cf-1", "cf-2"} { tk := makeTask(id) store.CreateTask(tk) pool.Submit(context.Background(), tk) - <-pool.Results() // drain + <-pool.Results() } + pool.mu.Lock() + draineEarly := pool.drained["claude"] + pool.mu.Unlock() + if draineEarly { + t.Error("expected claude NOT drained after only 2 failures") + } + + // Third failure should drain. + tk3 := makeTask("cf-3") + store.CreateTask(tk3) + pool.Submit(context.Background(), tk3) + <-pool.Results() pool.mu.Lock() drained := pool.drained["claude"] @@ -1564,18 +1576,18 @@ func TestPool_ConsecutiveFailures_DrainAtTwo(t *testing.T) { pool.mu.Unlock() if !drained { - t.Error("expected claude to be drained after 2 consecutive failures") + t.Error("expected claude to be drained after 3 consecutive failures") } - if failures < 2 { - t.Errorf("expected consecutiveFailures >= 2, got %d", failures) + if failures < 3 { + t.Errorf("expected consecutiveFailures >= 3, got %d", failures) } - // The second task should have a drain question set - tk2, err := store.GetTask("cf-2") + // The third task should have a drain question set. + tk3fetched, err := store.GetTask("cf-3") if err != nil { t.Fatalf("GetTask: %v", err) } - if tk2.QuestionJSON == "" { + if tk3fetched.QuestionJSON == "" { t.Error("expected drain question to be set on task after drain") } } |
