Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions internal/application/tickets/orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,8 @@ func (o *Orchestrator) StartFlow(ctx context.Context, ticketNumber string) error
}

state, loadErr := o.Store.LoadState(ticketNumber)
if os.IsNotExist(loadErr) {
if errors.Is(loadErr, os.ErrNotExist) {
state = workflowstate.New(ticketNumber)
saveErr := o.Store.SaveState(ticketNumber, state)
if saveErr != nil {
return fmt.Errorf("save initial ticket state: %w", saveErr)
}
} else if loadErr != nil {
return fmt.Errorf("load ticket state: %w", loadErr)
}
Expand Down Expand Up @@ -154,12 +150,8 @@ func (o *Orchestrator) MoveToState(ctx context.Context, ticketNumber, target str
}

state, loadErr := o.Store.LoadState(ticketNumber)
if os.IsNotExist(loadErr) {
if errors.Is(loadErr, os.ErrNotExist) {
state = workflowstate.New(ticketNumber)
saveErr := o.Store.SaveState(ticketNumber, state)
if saveErr != nil {
return fmt.Errorf("save initial ticket state: %w", saveErr)
}
} else if loadErr != nil {
return fmt.Errorf("load ticket state: %w", loadErr)
}
Expand Down
48 changes: 48 additions & 0 deletions internal/application/tickets/orchestrator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/Neokil/AutoPR/internal/config"
workflowstate "github.com/Neokil/AutoPR/internal/domain/workflowstate"
"github.com/Neokil/AutoPR/internal/providers"
statepkg "github.com/Neokil/AutoPR/internal/state"
)

// ── in-memory mocks ────────────────────────────────────────────────────────
Expand Down Expand Up @@ -169,6 +170,15 @@ func setupGitRepoWithBaseBranch(t *testing.T) string {
return root
}

func setupGitRepo(t *testing.T, root string) {
t.Helper()
runGit(t, root, "init", "-b", "main")
runGit(t, root, "config", "user.name", "Test User")
runGit(t, root, "config", "user.email", "test@example.com")
runGit(t, root, "add", ".")
runGit(t, root, "commit", "-m", "initial commit")
}

// ── StartFlow ─────────────────────────────────────────────────────────────

func TestStartFlow_newTicket_endsWaiting(t *testing.T) {
Expand Down Expand Up @@ -327,6 +337,44 @@ func TestStartFlow_writesBaseBranchIntoContextFiles(t *testing.T) {
}
}

func TestStartFlow_newTicketPersistsStateInWorktreeOnly(t *testing.T) {
t.Parallel()

root := setupRepo(t, minimalWorkflow, "Investigate this ticket.")
setupGitRepo(t, root)
store := statepkg.NewStore(root, ".auto-pr-state")
prov := &mockProvider{result: providers.ExecuteResult{RawOutput: "analysis done"}}
orch := tickets.NewWithStore(
config.Config{StateDirName: ".auto-pr-state", BaseBranch: "main"},
root,
store,
prov,
)

err := orch.StartFlow(context.Background(), "42")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

worktreeStatePath := filepath.Join(root, ".auto-pr-state", "worktrees", "42", ".auto-pr", statepkg.StateFileName)
_, statErr := os.Stat(worktreeStatePath)
if statErr != nil {
t.Fatalf("expected worktree-backed state file at %s: %v", worktreeStatePath, statErr)
}

legacyStatePath := filepath.Join(root, ".auto-pr-state", "42", statepkg.StateFileName)
_, statErr = os.Stat(legacyStatePath)
if !os.IsNotExist(statErr) {
t.Fatalf("expected no legacy state file at %s, got err=%v", legacyStatePath, statErr)
}

legacyDirPath := filepath.Join(root, ".auto-pr-state", "42")
_, statErr = os.Stat(legacyDirPath)
if !os.IsNotExist(statErr) {
t.Fatalf("expected no legacy ticket dir at %s, got err=%v", legacyDirPath, statErr)
}
}

func TestDiscoverTickets_persistsLogsUnderUserHome(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
Expand Down
11 changes: 9 additions & 2 deletions internal/server/job_failures.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package server

import (
"errors"
"fmt"
"os"
"strings"
Expand All @@ -16,10 +17,16 @@ func (s *server) persistTicketFailure(repoID, repoRoot, ticket string, repoRt *r

ticketState, err := repoRt.store.LoadState(ticket)
if err != nil {
if !os.IsNotExist(err) {
if !errors.Is(err, os.ErrNotExist) {
return fmt.Errorf("load ticket state: %w", err)
}
ticketState = workflowstate.New(ticket)

delErr := s.meta.DeleteTicket(repoID, ticket)
if delErr != nil {
return fmt.Errorf("delete ticket metadata: %w", delErr)
}

return nil
}

msg := strings.TrimSpace(cause.Error())
Expand Down
94 changes: 94 additions & 0 deletions internal/server/ticket_lifecycle_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package server //nolint:testpackage // needs access to unexported server internals

import (
"errors"
"os"
"path/filepath"
"testing"
"time"

"github.com/Neokil/AutoPR/internal/serverstate"
"github.com/Neokil/AutoPR/internal/state"
)

func newTestServer(t *testing.T, repoRoot string) (*server, *state.Store, string) {
t.Helper()

meta, err := serverstate.NewStore(filepath.Join(t.TempDir(), "server-state.json"))
if err != nil {
t.Fatalf("NewStore() error = %v", err)
}

store := state.NewStore(repoRoot, ".auto-pr-state")
repoID := "repo-1"

return &server{
meta: meta,
runtimes: map[string]*repoRuntime{
repoRoot: {
repoRoot: repoRoot,
store: store,
},
},
}, store, repoID
}

func TestEnsureQueuedTicketDoesNotPersistFreshState(t *testing.T) {
t.Parallel()

repoRoot := t.TempDir()
srv, store, repoID := newTestServer(t, repoRoot)

err := srv.ensureQueuedTicket(repoID, repoRoot, "GH-12")
if err != nil {
t.Fatalf("ensureQueuedTicket() error = %v", err)
}

_, loadErr := store.LoadState("GH-12")
if !errors.Is(loadErr, os.ErrNotExist) {
t.Fatalf("expected no persisted state, got err=%v", loadErr)
}

_, statErr := os.Stat(store.TicketDir("GH-12"))
if !os.IsNotExist(statErr) {
t.Fatalf("expected no legacy ticket dir, got err=%v", statErr)
}
}

func TestPersistTicketFailureWithoutStateDeletesMetadataOnly(t *testing.T) {
t.Parallel()

repoRoot := t.TempDir()
srv, store, repoID := newTestServer(t, repoRoot)
err := srv.meta.UpsertTicket(serverstate.TicketRecord{
RepoID: repoID,
RepoPath: repoRoot,
TicketNumber: "GH-12",
Status: "queued",
UpdatedAt: time.Now().UTC(),
})
if err != nil {
t.Fatalf("UpsertTicket() error = %v", err)
}

err = srv.persistTicketFailure(repoID, repoRoot, "GH-12", &repoRuntime{repoRoot: repoRoot, store: store}, queuedJob{
record: serverstate.JobRecord{Action: jobRun},
}, errors.New("worktree creation failed")) //nolint:err113 // test-local sentinel
if err != nil {
t.Fatalf("persistTicketFailure() error = %v", err)
}

_, loadErr := store.LoadState("GH-12")
if !errors.Is(loadErr, os.ErrNotExist) {
t.Fatalf("expected no persisted state, got err=%v", loadErr)
}

_, statErr := os.Stat(store.TicketDir("GH-12"))
if !os.IsNotExist(statErr) {
t.Fatalf("expected no legacy ticket dir, got err=%v", statErr)
}

if records := srv.meta.ListTickets(repoID); len(records) != 0 {
t.Fatalf("expected metadata to be removed, got %#v", records)
}
}
8 changes: 1 addition & 7 deletions internal/server/ticket_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,7 @@ func (s *server) ensureQueuedTicket(repoID, repoRoot, ticket string) error {
return fmt.Errorf("load ticket state: %w", loadErr)
}

st := workflowstate.New(ticket)
err = repoRt.store.SaveState(ticket, st)
if err != nil {
return fmt.Errorf("save initial ticket state: %w", err)
}

return s.syncTicketFromRepo(repoID, repoRoot, ticket, repoRt, true)
return nil
}

func (s *server) syncTicketFromRepo(repoID, repoRoot, ticket string, rt *repoRuntime, emitEvent bool) error {
Expand Down
11 changes: 6 additions & 5 deletions internal/state/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ func NewStore(repoRoot, stateDirName string) *Store {
}
}

// TicketDir returns the directory used to store state for the given ticket before a worktree exists.
// TicketDir returns the legacy pre-worktree directory for the given ticket.
func (s *Store) TicketDir(ticketNumber string) string {
return filepath.Join(s.StateRoot, ticketNumber)
}

// LoadState reads the persisted state for the ticket, preferring the worktree location
// when it exists and falling back to the pre-worktree state directory.
// and falling back to the legacy pre-worktree state directory for compatibility.
func (s *Store) LoadState(ticketNumber string) (workflowstate.State, error) {
// Prefer the worktree location when it exists.
wtStatePath := filepath.Join(s.worktreePath(ticketNumber), ".auto-pr", StateFileName)
Expand All @@ -54,7 +54,8 @@ func (s *Store) LoadState(ticketNumber string) (workflowstate.State, error) {
}

// SaveState persists st, writing to the worktree location once a worktree exists
// and removing the pre-worktree copy to keep a single source of truth.
// and removing the legacy copy to keep a single source of truth.
// Callers that save before a worktree exists still write to the legacy location.
func (s *Store) SaveState(ticketNumber string, state workflowstate.State) error {
state.Touch()
data, err := json.MarshalIndent(state, "", " ")
Expand Down Expand Up @@ -101,7 +102,7 @@ func (s *Store) ListTicketDirs() ([]string, error) {
seen := map[string]struct{}{}
var out []string

// Tickets with state still in the pre-worktree location.
// Tickets with state still in the legacy pre-worktree location.
entries, err := os.ReadDir(s.StateRoot)
if err != nil && !os.IsNotExist(err) {
return nil, fmt.Errorf("read state root: %w", err)
Expand Down Expand Up @@ -141,7 +142,7 @@ func (s *Store) ListTicketDirs() ([]string, error) {
return out, nil
}

// RemoveTicketDir deletes the pre-worktree state directory for the given ticket.
// RemoveTicketDir deletes the legacy pre-worktree state directory for the given ticket.
func (s *Store) RemoveTicketDir(ticketNumber string) error {
err := os.RemoveAll(s.TicketDir(ticketNumber))
if err != nil {
Expand Down
Loading
Loading