diff --git a/backend/internal/domain/lifecycle.go b/backend/internal/domain/lifecycle.go index 1727fad71..567a47693 100644 --- a/backend/internal/domain/lifecycle.go +++ b/backend/internal/domain/lifecycle.go @@ -20,10 +20,15 @@ const LifecycleVersion = 1 // between observations (they are read back by the pure decide core), so they // live in the persisted record too. type CanonicalSessionLifecycle struct { - Version int `json:"version"` - Session SessionSubstate `json:"session"` - PR PRSubstate `json:"pr"` - Runtime RuntimeSubstate `json:"runtime"` + // Version is the schema version of this record's shape (LifecycleVersion). + Version int `json:"version"` + // Revision is a monotonic counter the store bumps on every write. It is used + // for optimistic-concurrency checks (LifecyclePatch.ExpectedRevision) and is + // distinct from the schema Version above. + Revision int `json:"revision"` + Session SessionSubstate `json:"session"` + PR PRSubstate `json:"pr"` + Runtime RuntimeSubstate `json:"runtime"` // Activity is the last-known agent activity. It arrives on a different // cadence (ApplyActivitySignal) than runtime probes (the reaper), so the diff --git a/backend/internal/domain/session.go b/backend/internal/domain/session.go index f7761d99d..578cca406 100644 --- a/backend/internal/domain/session.go +++ b/backend/internal/domain/session.go @@ -17,17 +17,26 @@ const ( KindOrchestrator SessionKind = "orchestrator" ) -// Session is the read-model returned across the API boundary (to controllers, -// then the frontend). Status is the DERIVED display status, attached on read by -// the Session Manager so the API layer never recomputes it (single producer). -type Session struct { +// SessionRecord is the PERSISTENCE shape: identity, canonical lifecycle, and +// metadata — everything the store holds, and nothing derived. The store reads +// and writes records; it never produces the derived display status. +type SessionRecord struct { ID SessionID `json:"id"` ProjectID ProjectID `json:"projectId"` IssueID IssueID `json:"issueId,omitempty"` Kind SessionKind `json:"kind"` Lifecycle CanonicalSessionLifecycle `json:"lifecycle"` - Status SessionStatus `json:"status"` Metadata map[string]string `json:"metadata,omitempty"` CreatedAt time.Time `json:"createdAt"` UpdatedAt time.Time `json:"updatedAt"` } + +// Session is the read-model returned across the API boundary (to controllers, +// then the frontend): a SessionRecord plus the DERIVED display Status. The +// Session Manager is the single producer of Status — it builds a Session from a +// stored SessionRecord by calling DeriveLegacyStatus, so the store and API +// never recompute (or accidentally persist) it. +type Session struct { + SessionRecord + Status SessionStatus `json:"status"` +} diff --git a/backend/internal/domain/status.go b/backend/internal/domain/status.go index 7eed758ac..b12b2b9f6 100644 --- a/backend/internal/domain/status.go +++ b/backend/internal/domain/status.go @@ -28,12 +28,17 @@ const ( // DeriveLegacyStatus is the ONLY producer of the display status. It must stay a // pure, total function of the canonical record. // -// Order matters and encodes the core invariant "PR facts dominate session facts -// once a PR exists": -// 1. Terminal / hard session states map directly (terminated sub-switches on reason). -// 2. A merged PR wins. -// 3. An open PR maps by its reason. -// 4. Otherwise fall through to the raw session state. +// Order matters: +// 1. Terminal / hard session states (done, terminated, needs_input, stuck, +// detecting, not_started) map directly — these OUTRANK PR facts. +// 2. Otherwise a merged PR wins. +// 3. Otherwise an open PR maps by its reason. +// 4. Otherwise fall through to the SOFT session state (idle/working). +// +// So "PR facts dominate session facts" applies only to the soft states: an idle +// or working session with an open, CI-failing PR displays as ci_failed — but a +// session that is stuck or needs_input shows that regardless of PR state, since +// it needs a human either way. func DeriveLegacyStatus(l CanonicalSessionLifecycle) SessionStatus { switch l.Session.State { case SessionDone: diff --git a/backend/internal/ports/facts.go b/backend/internal/ports/facts.go index b8d9eaaf9..55f4f6ca6 100644 --- a/backend/internal/ports/facts.go +++ b/backend/internal/ports/facts.go @@ -2,8 +2,8 @@ // lane: the inbound interfaces we implement, the outbound interfaces others // implement for us, and the fact DTOs that cross those boundaries. // -// These are the types adil (SCM poller), Tom (persistence), and aditi (API) -// build against, so they are committed and stabilised before the LCM/SM logic. +// These are the types the SCM poller, persistence adapter, and API layer build +// against, so they are committed and stabilised before the LCM/SM logic. package ports import ( @@ -12,7 +12,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) -// SCMFacts is produced by adil's SCM poller and handed to ApplySCMObservation. +// SCMFacts is produced by the SCM poller and handed to ApplySCMObservation. // // Fetched is the failed-probe guard: when false, the GitHub query timed out or // errored and the rest of the struct is meaningless — the LCM must NOT read it @@ -120,10 +120,12 @@ const ( ) // SpawnOutcome is what the Session Manager reports to the LCM after a spawn. +// RuntimeHandle is the same structured handle the Runtime port returns, so no +// ad-hoc string encoding is needed for later Destroy/SendMessage calls. type SpawnOutcome struct { Branch string WorkspacePath string - RuntimeHandle string + RuntimeHandle RuntimeHandle AgentSessionID string } diff --git a/backend/internal/ports/outbound.go b/backend/internal/ports/outbound.go index 2ad91cb99..7a3649ae5 100644 --- a/backend/internal/ports/outbound.go +++ b/backend/internal/ports/outbound.go @@ -6,33 +6,41 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) -// LifecycleStore is Tom's persistence layer, the ONLY disk writer. It owns +// LifecycleStore is the persistence adapter, the ONLY disk writer. It owns // merge-patch, atomic write, file lock, and CDC eventing. The LCM and SM only // ever touch state through this narrow interface. +// +// List returns persistence records (no derived status); the Session Manager +// turns those into domain.Session by attaching the derived display status. type LifecycleStore interface { Load(ctx context.Context, id domain.SessionID) (domain.CanonicalSessionLifecycle, bool, error) PatchLifecycle(ctx context.Context, id domain.SessionID, patch LifecyclePatch) error - List(ctx context.Context, project domain.ProjectID) ([]domain.Session, error) + List(ctx context.Context, project domain.ProjectID) ([]domain.SessionRecord, error) GetMetadata(ctx context.Context, id domain.SessionID) (map[string]string, error) PatchMetadata(ctx context.Context, id domain.SessionID, kv map[string]string) error } // LifecyclePatch is a sparse merge-patch: a nil field is left untouched, a -// non-nil field is written. Detecting needs three-way semantics (leave / set / -// clear-to-nil) which a single pointer can't express, so ClearDetecting handles -// the clear case explicitly. +// non-nil field is written. // -// ExpectedVersion supports optimistic concurrency: when non-nil the store must -// reject the patch if the stored Version differs. (Open for Tom to confirm vs. -// the LCM owning all serialisation itself.) +// Detecting needs three-way semantics (leave / set / clear-to-nil): +// - ClearDetecting == true → store clears the detecting memory and IGNORES +// the Detecting field (clear wins; setting both is a caller bug). +// - ClearDetecting == false, Detecting != nil → set/replace the memory. +// - ClearDetecting == false, Detecting == nil → leave it untouched. +// +// ExpectedRevision supports optimistic concurrency: when non-nil the store must +// reject the patch if the stored Revision (the monotonic write counter, NOT the +// schema Version) differs. This is the alternative to the LCM owning all +// per-session serialisation itself. type LifecyclePatch struct { - Session *domain.SessionSubstate - PR *domain.PRSubstate - Runtime *domain.RuntimeSubstate - Activity *domain.ActivitySubstate - Detecting *domain.DetectingState - ClearDetecting bool - ExpectedVersion *int + Session *domain.SessionSubstate + PR *domain.PRSubstate + Runtime *domain.RuntimeSubstate + Activity *domain.ActivitySubstate + Detecting *domain.DetectingState + ClearDetecting bool + ExpectedRevision *int } // Notifier delivers events to the human (desktop/Slack later). Push, never pull. @@ -92,7 +100,10 @@ type RuntimeHandle struct { type Agent interface { GetLaunchCommand(cfg AgentConfig) string GetEnvironment(cfg AgentConfig) map[string]string - IsProcessRunning(ctx context.Context, handle RuntimeHandle) (domain.ActivityState, error) + // ProbeProcess returns the agent process liveness classification + // (alive/dead/indeterminate/failed) — not a boolean and not an activity + // state. Activity classification arrives separately via ActivitySignal. + ProbeProcess(ctx context.Context, handle RuntimeHandle) (ProcessProbe, error) GetRestoreCommand(agentSessionID string) string }