refactor: move session status assembly to service (#62)

Author: i-trytoohard

README.md

@@ -4,7 +4,7 @@ Rewrite of the agent-orchestrator: a long-running Go backend daemon (`backend/`)
 paired with a placeholder Electron + TypeScript frontend shell (`frontend/`).
 
 See [`docs/`](docs/README.md) for architecture and status — start with the
-Lifecycle Manager + Session Manager lane in [`docs/architecture.md`](docs/architecture.md).
+Lifecycle Manager + Session Service lane in [`docs/architecture.md`](docs/architecture.md).
 
 ## Backend daemon
 

backend/internal/domain/status.go

@@ -19,52 +19,3 @@ const (
 	StatusIdle             SessionStatus = "idle"
 	StatusTerminated       SessionStatus = "terminated"
 )
-
-// DeriveStatus is the ONLY producer of display status. It is a pure function of
-// persisted session facts and PR facts: is_terminated, activity_state, and the PR
-// table are the durable facts that tell the UI what it needs to know.
-func DeriveStatus(rec SessionRecord, pr *PRFacts) SessionStatus {
-	if rec.IsTerminated {
-		if pr != nil && pr.Merged {
-			return StatusMerged
-		}
-		return StatusTerminated
-	}
-
-	if rec.Activity.State == ActivityWaitingInput {
-		return StatusNeedsInput
-	}
-
-	if pr != nil {
-		if pr.Merged {
-			return StatusMerged
-		}
-		if !pr.Closed {
-			return prPipelineStatus(*pr)
-		}
-	}
-
-	if rec.Activity.State == ActivityActive {
-		return StatusWorking
-	}
-	return StatusIdle
-}
-
-func prPipelineStatus(pr PRFacts) SessionStatus {
-	switch {
-	case pr.CI == CIFailing:
-		return StatusCIFailed
-	case pr.Draft:
-		return StatusDraft
-	case pr.Review == ReviewChangesRequest || pr.ReviewComments:
-		return StatusChangesRequested
-	case pr.Mergeability == MergeMergeable:
-		return StatusMergeable
-	case pr.Review == ReviewApproved:
-		return StatusApproved
-	case pr.Review == ReviewRequired:
-		return StatusReviewPending
-	default:
-		return StatusPROpen
-	}
-}

backend/internal/domain/status_test.go

@@ -11,7 +11,8 @@ import (
 	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
 	prsvc "github.com/aoagents/agent-orchestrator/backend/internal/pr"
 	"github.com/aoagents/agent-orchestrator/backend/internal/project"
-	"github.com/aoagents/agent-orchestrator/backend/internal/session"
+	"github.com/aoagents/agent-orchestrator/backend/internal/service"
+	sessionmanager "github.com/aoagents/agent-orchestrator/backend/internal/session_manager"
 	"github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite"
 )
 
@@ -54,7 +55,7 @@ func (c *captureMessenger) Send(_ context.Context, _ domain.SessionID, msg strin
 
 type stack struct {
 	store *sqlite.Store
-	sm    *session.Manager
+	sm    *service.Session
 	lcm   *lifecycle.Manager
 	prm   *prsvc.Manager
 	rt    *stubRuntime
@@ -78,7 +79,8 @@ func newStack(t *testing.T) *stack {
 	prm := prsvc.New(prsvc.Deps{Writer: store, Lifecycle: lcm})
 	rt := &stubRuntime{}
 	ws := &stubWorkspace{}
-	sm := session.New(session.Deps{Runtime: rt, Agent: stubAgent{}, Workspace: ws, Store: store, Messenger: msg, Lifecycle: lcm})
+	mgr := sessionmanager.New(sessionmanager.Deps{Runtime: rt, Agent: stubAgent{}, Workspace: ws, Store: store, Messenger: msg, Lifecycle: lcm})
+	sm := service.NewSession(mgr, store)
 	return &stack{store: store, sm: sm, lcm: lcm, prm: prm, rt: rt, ws: ws, msg: msg}
 }
 

backend/internal/integration/lifecycle_sqlite_test.go

@@ -0,0 +1,102 @@
+package service
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
+	"github.com/aoagents/agent-orchestrator/backend/internal/ports"
+	sessionmanager "github.com/aoagents/agent-orchestrator/backend/internal/session_manager"
+)
+
+// SessionStore is the persistence surface needed to assemble controller-facing session read models.
+type SessionStore interface {
+	sessionmanager.Store
+	GetDisplayPRFactsForSession(ctx context.Context, id domain.SessionID) (domain.PRFacts, bool, error)
+}
+
+// Session is the controller-facing session service. It delegates command-side
+// session operations to the internal sessionmanager.Manager and owns read-model
+// assembly, including user-facing display status derivation.
+type Session struct {
+	manager *sessionmanager.Manager
+	store   SessionStore
+}
+
+// NewSession wires a controller-facing session service over an internal session Manager.
+func NewSession(manager *sessionmanager.Manager, store SessionStore) *Session {
+	return &Session{manager: manager, store: store}
+}
+
+// Spawn creates a session and returns the API-facing read model.
+func (s *Session) Spawn(ctx context.Context, cfg ports.SpawnConfig) (domain.Session, error) {
+	rec, err := s.manager.Spawn(ctx, cfg)
+	if err != nil {
+		return domain.Session{}, err
+	}
+	return s.toSession(ctx, rec)
+}
+
+// Restore relaunches a terminated session and returns the API-facing read model.
+func (s *Session) Restore(ctx context.Context, id domain.SessionID) (domain.Session, error) {
+	rec, err := s.manager.Restore(ctx, id)
+	if err != nil {
+		return domain.Session{}, err
+	}
+	return s.toSession(ctx, rec)
+}
+
+// Kill delegates terminal intent and teardown to the internal manager.
+func (s *Session) Kill(ctx context.Context, id domain.SessionID) (bool, error) {
+	return s.manager.Kill(ctx, id)
+}
+
+// Send delegates agent messaging to the internal manager.
+func (s *Session) Send(ctx context.Context, id domain.SessionID, message string) error {
+	return s.manager.Send(ctx, id, message)
+}
+
+// Cleanup delegates terminal workspace cleanup to the internal manager.
+func (s *Session) Cleanup(ctx context.Context, project domain.ProjectID) ([]domain.SessionID, error) {
+	return s.manager.Cleanup(ctx, project)
+}
+
+// List returns the project's sessions as enriched display models.
+func (s *Session) List(ctx context.Context, project domain.ProjectID) ([]domain.Session, error) {
+	recs, err := s.store.ListSessions(ctx, project)
+	if err != nil {
+		return nil, fmt.Errorf("list %s: %w", project, err)
+	}
+	out := make([]domain.Session, 0, len(recs))
+	for _, rec := range recs {
+		sess, err := s.toSession(ctx, rec)
+		if err != nil {
+			return nil, err
+		}
+		out = append(out, sess)
+	}
+	return out, nil
+}
+
+// Get returns one session as an enriched display model, or sessionmanager.ErrNotFound if it is absent.
+func (s *Session) Get(ctx context.Context, id domain.SessionID) (domain.Session, error) {
+	rec, ok, err := s.store.GetSession(ctx, id)
+	if err != nil {
+		return domain.Session{}, fmt.Errorf("get %s: %w", id, err)
+	}
+	if !ok {
+		return domain.Session{}, fmt.Errorf("get %s: %w", id, sessionmanager.ErrNotFound)
+	}
+	return s.toSession(ctx, rec)
+}
+
+func (s *Session) toSession(ctx context.Context, rec domain.SessionRecord) (domain.Session, error) {
+	pr, ok, err := s.store.GetDisplayPRFactsForSession(ctx, rec.ID)
+	if err != nil {
+		return domain.Session{}, fmt.Errorf("pr facts %s: %w", rec.ID, err)
+	}
+	if !ok {
+		return domain.Session{SessionRecord: rec, Status: deriveStatus(rec, nil)}, nil
+	}
+	return domain.Session{SessionRecord: rec, Status: deriveStatus(rec, &pr)}, nil
+}

backend/internal/service/session.go

@@ -0,0 +1,49 @@
+package service
+
+import "github.com/aoagents/agent-orchestrator/backend/internal/domain"
+
+func deriveStatus(rec domain.SessionRecord, pr *domain.PRFacts) domain.SessionStatus {
+	if rec.IsTerminated {
+		if pr != nil && pr.Merged {
+			return domain.StatusMerged
+		}
+		return domain.StatusTerminated
+	}
+
+	if rec.Activity.State == domain.ActivityWaitingInput {
+		return domain.StatusNeedsInput
+	}
+
+	if pr != nil {
+		if pr.Merged {
+			return domain.StatusMerged
+		}
+		if !pr.Closed {
+			return prPipelineStatus(*pr)
+		}
+	}
+
+	if rec.Activity.State == domain.ActivityActive {
+		return domain.StatusWorking
+	}
+	return domain.StatusIdle
+}
+
+func prPipelineStatus(pr domain.PRFacts) domain.SessionStatus {
+	switch {
+	case pr.CI == domain.CIFailing:
+		return domain.StatusCIFailed
+	case pr.Draft:
+		return domain.StatusDraft
+	case pr.Review == domain.ReviewChangesRequest || pr.ReviewComments:
+		return domain.StatusChangesRequested
+	case pr.Mergeability == domain.MergeMergeable:
+		return domain.StatusMergeable
+	case pr.Review == domain.ReviewApproved:
+		return domain.StatusApproved
+	case pr.Review == domain.ReviewRequired:
+		return domain.StatusReviewPending
+	default:
+		return domain.StatusPROpen
+	}
+}

backend/internal/service/session_status.go

@@ -0,0 +1,42 @@
+package service
+
+import (
+	"testing"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
+)
+
+func statusRec(activity domain.ActivityState, terminated bool) domain.SessionRecord {
+	return domain.SessionRecord{Activity: domain.Activity{State: activity}, IsTerminated: terminated}
+}
+
+func statusPR(facts domain.PRFacts) *domain.PRFacts { return &facts }
+
+func TestServiceDerivesStatusFromSessionFactsAndPR(t *testing.T) {
+	tests := []struct {
+		name string
+		rec  domain.SessionRecord
+		pr   *domain.PRFacts
+		want domain.SessionStatus
+	}{
+		{"terminated", statusRec(domain.ActivityExited, true), nil, domain.StatusTerminated},
+		{"merged-pr", statusRec(domain.ActivityIdle, true), statusPR(domain.PRFacts{Merged: true}), domain.StatusMerged},
+		{"needs-input", statusRec(domain.ActivityWaitingInput, false), statusPR(domain.PRFacts{CI: domain.CIFailing}), domain.StatusNeedsInput},
+		{"ci-failed", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{CI: domain.CIFailing}), domain.StatusCIFailed},
+		{"draft", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Draft: true}), domain.StatusDraft},
+		{"changes-requested", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewChangesRequest}), domain.StatusChangesRequested},
+		{"mergeable", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Mergeability: domain.MergeMergeable}), domain.StatusMergeable},
+		{"approved", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewApproved}), domain.StatusApproved},
+		{"review-pending", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewRequired}), domain.StatusReviewPending},
+		{"pr-open", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{}), domain.StatusPROpen},
+		{"working", statusRec(domain.ActivityActive, false), nil, domain.StatusWorking},
+		{"idle", statusRec(domain.ActivityIdle, false), nil, domain.StatusIdle},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := deriveStatus(tt.rec, tt.pr); got != tt.want {
+				t.Fatalf("got %q want %q", got, tt.want)
+			}
+		})
+	}
+}

backend/internal/service/session_status_test.go

@@ -0,0 +1,60 @@
+package service
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/aoagents/agent-orchestrator/backend/internal/domain"
+)
+
+type fakeSessionStore struct {
+	sessions map[domain.SessionID]domain.SessionRecord
+	pr       map[domain.SessionID]domain.PRFacts
+	num      int
+}
+
+func newFakeSessionStore() *fakeSessionStore {
+	return &fakeSessionStore{sessions: map[domain.SessionID]domain.SessionRecord{}, pr: map[domain.SessionID]domain.PRFacts{}}
+}
+
+func (f *fakeSessionStore) CreateSession(_ context.Context, rec domain.SessionRecord) (domain.SessionRecord, error) {
+	f.num++
+	rec.ID = domain.SessionID(fmt.Sprintf("%s-%d", rec.ProjectID, f.num))
+	f.sessions[rec.ID] = rec
+	return rec, nil
+}
+
+func (f *fakeSessionStore) GetSession(_ context.Context, id domain.SessionID) (domain.SessionRecord, bool, error) {
+	r, ok := f.sessions[id]
+	return r, ok, nil
+}
+
+func (f *fakeSessionStore) ListSessions(_ context.Context, p domain.ProjectID) ([]domain.SessionRecord, error) {
+	var out []domain.SessionRecord
+	for _, r := range f.sessions {
+		if r.ProjectID == p {
+			out = append(out, r)
+		}
+	}
+	return out, nil
+}
+
+func (f *fakeSessionStore) GetDisplayPRFactsForSession(_ context.Context, id domain.SessionID) (domain.PRFacts, bool, error) {
+	pr, ok := f.pr[id]
+	return pr, ok, nil
+}
+
+func TestSessionListDerivesStatusFromPRFacts(t *testing.T) {
+	st := newFakeSessionStore()
+	st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Activity: domain.Activity{State: domain.ActivityActive}}
+	st.pr["mer-1"] = domain.PRFacts{URL: "pr1", CI: domain.CIFailing}
+
+	list, err := (&Session{store: st}).List(context.Background(), "mer")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(list) != 1 || list[0].Status != domain.StatusCIFailed {
+		t.Fatalf("got %+v", list)
+	}
+}