diff --git a/backend/internal/adapters/agent/activitydispatch/dispatch.go b/backend/internal/adapters/agent/activitydispatch/dispatch.go index 812e15e2..8464f941 100644 --- a/backend/internal/adapters/agent/activitydispatch/dispatch.go +++ b/backend/internal/adapters/agent/activitydispatch/dispatch.go @@ -61,7 +61,7 @@ func Derive(agent, event string, payload []byte) (domain.ActivityState, bool) { // SupportsHarness reports whether a harness has an activity pipeline at all: // a registered deriver here means its adapter installs `ao hooks ` // callbacks that can reach the daemon. Status derivation uses this to decide -// whether prolonged silence is suspicious (no_signal) or simply all a hook-less +// whether prolonged silence is suspicious (a broken-pipeline stall) or simply all a hook-less // harness can ever report (idle). Harness names and `ao hooks` agent tokens are // the same strings by convention. func SupportsHarness(h domain.AgentHarness) bool { diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index 5d79ec31..f55d0288 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -107,8 +107,8 @@ func startSession(cfg config.Config, runtime *zellij.Runtime, store *sqlite.Stor PRClaimer: store, SCM: scmProvider, Telemetry: telemetry, - // no_signal only makes sense for harnesses whose adapters install - // activity hooks; the deriver registry is the source of truth for that. + // The broken-pipeline stall only makes sense for harnesses whose adapters + // install activity hooks; the deriver registry is the source of truth. SignalCapable: activitydispatch.SupportsHarness, }) // Triggering a review spawns a reviewer over the worker's worktree, resolved diff --git a/backend/internal/domain/session.go b/backend/internal/domain/session.go index 6cc639e5..6ea1a0ef 100644 --- a/backend/internal/domain/session.go +++ b/backend/internal/domain/session.go @@ -46,7 +46,7 @@ type SessionRecord struct { // FirstSignalAt is when the FIRST agent hook callback arrived for the // current spawn/restore: raw signal receipt, independent of the derived // activity state. Zero means no hook has ever reported, which deriveStatus - // surfaces as StatusNoSignal after a grace period. Internal fact, not part + // surfaces as StatusStalled after a boot grace. Internal fact, not part // of the API read model. FirstSignalAt time.Time `json:"-"` IsTerminated bool `json:"isTerminated"` diff --git a/backend/internal/domain/status.go b/backend/internal/domain/status.go index 4b6cf254..8afaf399 100644 --- a/backend/internal/domain/status.go +++ b/backend/internal/domain/status.go @@ -2,25 +2,26 @@ package domain // SessionStatus is the single-word DISPLAY status the dashboard renders. It is // derived from persisted session facts plus PR facts and is never stored. +// +// There are five states, one per distinct move a human makes when scanning a +// wall of agents: leave it alone, respond, act on a clean PR, get it moving, or +// nothing. Finer PR detail (CI failing vs changes requested vs approved) lives +// in the inspector, not in the glanceable status. type SessionStatus string // The display statuses the dashboard renders. const ( - StatusWorking SessionStatus = "working" - StatusPROpen SessionStatus = "pr_open" - StatusDraft SessionStatus = "draft" - StatusCIFailed SessionStatus = "ci_failed" - StatusReviewPending SessionStatus = "review_pending" - StatusChangesRequested SessionStatus = "changes_requested" - StatusApproved SessionStatus = "approved" - StatusMergeable SessionStatus = "mergeable" - StatusMerged SessionStatus = "merged" - StatusNeedsInput SessionStatus = "needs_input" - StatusIdle SessionStatus = "idle" - StatusTerminated SessionStatus = "terminated" - // StatusNoSignal marks a live session whose agent has never delivered a - // hook callback for the current spawn/restore: AO cannot tell whether the - // agent is working or stuck (broken hook pipeline, blocked interactive - // prompt). Rendered instead of a confident idle. - StatusNoSignal SessionStatus = "no_signal" + // StatusWorking — the agent is actively running. Leave it alone. + StatusWorking SessionStatus = "working" + // StatusNeedsInput — the agent is blocked on you. Respond. + StatusNeedsInput SessionStatus = "needs_input" + // StatusReady — a clean PR is waiting on you (mergeable, approved, or needs + // your review). Merge it / go review it. + StatusReady SessionStatus = "ready" + // StatusStalled — the agent will not finish on its own (hung, never booted, + // or stopped with unfinished work). Get it moving. + StatusStalled SessionStatus = "stalled" + // StatusIdle — nothing is happening, or the work is finished (also covers + // merged and terminated). Nothing to do. + StatusIdle SessionStatus = "idle" ) diff --git a/backend/internal/httpd/controllers/sessions_test.go b/backend/internal/httpd/controllers/sessions_test.go index 1b588680..4b0dde33 100644 --- a/backend/internal/httpd/controllers/sessions_test.go +++ b/backend/internal/httpd/controllers/sessions_test.go @@ -92,7 +92,7 @@ func (f *fakeSessionService) Restore(_ context.Context, id domain.SessionID) (do func (f *fakeSessionService) Kill(_ context.Context, id domain.SessionID) (bool, error) { s := f.sessions[id] s.IsTerminated = true - s.Status = domain.StatusTerminated + s.Status = domain.StatusIdle f.sessions[id] = s return true, nil } diff --git a/backend/internal/integration/lifecycle_sqlite_test.go b/backend/internal/integration/lifecycle_sqlite_test.go index 9a0a9960..153e0076 100644 --- a/backend/internal/integration/lifecycle_sqlite_test.go +++ b/backend/internal/integration/lifecycle_sqlite_test.go @@ -123,8 +123,8 @@ func TestSpawnPRKillRoundTrip(t *testing.T) { if err != nil { t.Fatal(err) } - if got.Status != domain.StatusCIFailed { - t.Fatalf("want ci_failed, got %q", got.Status) + if got.Status != domain.StatusStalled { + t.Fatalf("want stalled, got %q", got.Status) } freed, err := st.sm.Kill(ctx, sess.ID) if err != nil || !freed { diff --git a/backend/internal/lifecycle/manager.go b/backend/internal/lifecycle/manager.go index 6212d8eb..56b7bfe1 100644 --- a/backend/internal/lifecycle/manager.go +++ b/backend/internal/lifecycle/manager.go @@ -136,7 +136,7 @@ func (m *Manager) ApplyActivitySignal(ctx context.Context, id domain.SessionID, act := domain.Activity{State: s.State, LastActivityAt: timeOr(s.Timestamp, now)} // A same-state repeat is still a write when it is the FIRST signal for // this spawn: the receipt itself is a durable fact (it clears the - // no_signal display status). Hook deliveries are best-effort, so the + // never-booted stalled status). Hook deliveries are best-effort, so the // first to ARRIVE may match the seeded state — e.g. a turn's "active" // POST is lost and its Stop hook lands idle on the idle-seeded row. if sameActivity(rec.Activity, act) && !rec.FirstSignalAt.IsZero() { @@ -243,8 +243,8 @@ func (m *Manager) MarkSpawned(ctx context.Context, id domain.SessionID, metadata rec.IsTerminated = false rec.Activity = domain.Activity{State: domain.ActivityIdle, LastActivityAt: now} // Each spawn/restore must re-prove its hook pipeline: clear the receipt so - // a relaunch with broken hooks degrades to no_signal instead of inheriting - // a stale "signals worked once" fact. + // a relaunch with broken hooks degrades to a never-booted stall instead of + // inheriting a stale "signals worked once" fact. rec.FirstSignalAt = time.Time{} rec.Metadata = mergeMetadata(rec.Metadata, metadata) rec.UpdatedAt = now diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index bbcb32c1..5f1e0b90 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -84,8 +84,8 @@ type Service struct { telemetry ports.EventSink // signalCapable reports whether a harness has a hook pipeline that can // deliver activity signals at all. Only capable harnesses are eligible for - // the no_signal downgrade — a hook-less harness staying silent forever is - // normal, not a broken pipeline. nil means "unknown": never downgrade. + // the never-booted stall downgrade: a hook-less harness staying silent + // forever is normal, not a broken pipeline. nil means "unknown": never stall. signalCapable func(domain.AgentHarness) bool } @@ -104,9 +104,9 @@ type Deps struct { SCM scmProvider Clock func() time.Time Telemetry ports.EventSink - // SignalCapable gates the no_signal status downgrade per harness; daemon + // SignalCapable gates the never-booted stall downgrade per harness; daemon // wiring passes activitydispatch.SupportsHarness. Left nil, no session is - // ever downgraded to no_signal. + // ever stalled for silence. SignalCapable func(domain.AgentHarness) bool } @@ -484,7 +484,7 @@ func (s *Service) now() time.Time { // harnessSignals tolerates a zero-value Service the same way now does. Without // an injected capability predicate the service cannot tell a broken pipeline -// from a hook-less harness, so it never claims no_signal. +// from a hook-less harness, so it never stalls a session for silence. func (s *Service) harnessSignals(h domain.AgentHarness) bool { if s.signalCapable == nil { return false diff --git a/backend/internal/service/session/service_test.go b/backend/internal/service/session/service_test.go index ee114108..16b8bf18 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -104,14 +104,16 @@ func (f *fakeStore) GetProject(_ context.Context, id string) (domain.ProjectReco func TestSessionListDerivesStatusFromPRFacts(t *testing.T) { st := newFakeStore() - st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Activity: domain.Activity{State: domain.ActivityActive}} + // Stopped agent on a failing-CI PR: it had the move and quit, so the + // session reads Stalled (PR facts drive the status, not the activity). + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Activity: domain.Activity{State: domain.ActivityIdle}} st.pr["mer-1"] = domain.PRFacts{URL: "pr1", CI: domain.CIFailing} list, err := (&Service{store: st}).List(context.Background(), ListFilter{ProjectID: "mer"}) if err != nil { t.Fatal(err) } - if len(list) != 1 || list[0].Status != domain.StatusCIFailed { + if len(list) != 1 || list[0].Status != domain.StatusStalled { t.Fatalf("got %+v", list) } } diff --git a/backend/internal/service/session/stack_test.go b/backend/internal/service/session/stack_test.go index c2d2f0cc..be9787ff 100644 --- a/backend/internal/service/session/stack_test.go +++ b/backend/internal/service/session/stack_test.go @@ -42,59 +42,59 @@ func TestBuildStacksMergedParentUnblocksChild(t *testing.T) { } } -func TestDeriveStatusWorstWinsAcrossIndependentPRs(t *testing.T) { - // Two independent open PRs (both target main): mergeable vs ci_failed. - // CI failure is more urgent, so the session reports ci_failed. +func TestDeriveStatusUnfinishedPRWinsAcrossIndependentPRs(t *testing.T) { + // Two independent open PRs: one mergeable (clean), one ci_failed + // (unfinished). Stopped on undone work outranks the clean PR → Stalled. prs := []domain.PRFacts{ {URL: "a", SourceBranch: "ao/a", TargetBranch: "main", Mergeability: domain.MergeMergeable}, {URL: "b", SourceBranch: "ao/b", TargetBranch: "main", CI: domain.CIFailing}, } - if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusCIFailed { - t.Fatalf("got %q want ci_failed", got) + if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusStalled { + t.Fatalf("got %q want stalled", got) } } -func TestDeriveStatusAllMergeableReportsMergeable(t *testing.T) { +func TestDeriveStatusAllCleanIsReady(t *testing.T) { prs := []domain.PRFacts{ {URL: "a", SourceBranch: "ao/a", TargetBranch: "main", Mergeability: domain.MergeMergeable}, {URL: "b", SourceBranch: "ao/b", TargetBranch: "main", Mergeability: domain.MergeMergeable}, } - if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusMergeable { - t.Fatalf("got %q want mergeable", got) + if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusReady { + t.Fatalf("got %q want ready", got) } } -func TestDeriveStatusStackedChildExemptFromAggregation(t *testing.T) { - // Root mergeable; blocked child is pr_open. Child is exempt, so the session - // reports mergeable rather than being dragged down to pr_open. +func TestDeriveStatusStackedChildExemptFromReadiness(t *testing.T) { + // Root mergeable (clean); blocked child is a bare open PR (neutral). The + // child contributes nothing, so the session reads Ready off the root. prs := []domain.PRFacts{ {URL: "root", SourceBranch: "ao/abc", TargetBranch: "main", Mergeability: domain.MergeMergeable}, {URL: "child", SourceBranch: "ao/abc/x", TargetBranch: "ao/abc"}, } - if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusMergeable { - t.Fatalf("got %q want mergeable (child exempt)", got) + if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusReady { + t.Fatalf("got %q want ready (child neutral)", got) } } -func TestDeriveStatusMergedParentOpenChildStaysOnChild(t *testing.T) { - // Parent merged, child now unblocked and review_pending: still alive, status - // follows the open child. +func TestDeriveStatusMergedParentOpenCleanChildIsReady(t *testing.T) { + // Parent merged, child now unblocked and review_required (clean): stopped on + // a clean PR reads Ready. prs := []domain.PRFacts{ {URL: "root", SourceBranch: "ao/abc", TargetBranch: "main", Merged: true}, {URL: "child", SourceBranch: "ao/abc/x", TargetBranch: "main", Review: domain.ReviewRequired}, } - if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusReviewPending { - t.Fatalf("got %q want review_pending", got) + if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusReady { + t.Fatalf("got %q want ready", got) } } -func TestDeriveStatusAllMergedReportsMerged(t *testing.T) { +func TestDeriveStatusAllMergedIsIdle(t *testing.T) { prs := []domain.PRFacts{ {URL: "a", Merged: true}, {URL: "b", Merged: true}, } - if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusMerged { - t.Fatalf("got %q want merged", got) + if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusIdle { + t.Fatalf("got %q want idle", got) } } @@ -114,14 +114,15 @@ func TestDeriveStatusEmptyPRsUsesActivity(t *testing.T) { } } -func TestDeriveStatusDegenerateAllBlockedStillAggregates(t *testing.T) { - // Two PRs each targeting the other's source branch (no visible root). The - // fallback aggregates across all so the session never goes dark. +func TestDeriveStatusDegenerateAllBlockedSurfacesUnfinished(t *testing.T) { + // Two PRs each targeting the other's source branch (no visible root). Both + // are blocked, but a failing-CI child surfaces as unfinished work, so the + // stopped session reads Stalled rather than going dark. prs := []domain.PRFacts{ {URL: "a", SourceBranch: "x", TargetBranch: "y", CI: domain.CIFailing}, {URL: "b", SourceBranch: "y", TargetBranch: "x", Mergeability: domain.MergeMergeable}, } - if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusCIFailed { - t.Fatalf("got %q want ci_failed (degenerate fallback)", got) + if got := deriveStatus(live(), prs, statusNow, true); got != domain.StatusStalled { + t.Fatalf("got %q want stalled (degenerate, unfinished surfaces)", got) } } diff --git a/backend/internal/service/session/status.go b/backend/internal/service/session/status.go index 1bacbbde..f7c96752 100644 --- a/backend/internal/service/session/status.go +++ b/backend/internal/service/session/status.go @@ -6,55 +6,67 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) -// noSignalGrace is how long after spawn/restore a session may stay silent -// before its idle reading is downgraded to StatusNoSignal. It covers the -// agent's TUI boot plus the gap to the first activity-bearing hook callback -// (for Codex that is UserPromptSubmit, seconds after the auto-submitted spawn -// prompt — its SessionStart hook fires earlier but carries no activity state); -// past it, a silent session is indistinguishable from one with a broken hook -// pipeline, and the dashboard must not claim a confident "idle". -const noSignalGrace = 90 * time.Second +// bootGrace is how long after spawn/restore a signal-capable session may stay +// silent before its silence reads as a broken pipeline (never booted) rather +// than a normal boot gap. It covers the agent's TUI boot plus the gap to the +// first activity-bearing hook callback. +const bootGrace = 120 * time.Second -// deriveStatus computes the display status. signalCapable says whether this -// session's harness has an activity hook pipeline at all; only then can -// prolonged silence mean the pipeline is broken (no_signal) rather than the -// permanent, normal silence of a hook-less harness. +// hangTimeout is how long an "active" session may go silent before it reads as +// hung mid-run (wedged on a tool call or rate limit) instead of working. The +// detector trades false positives for catching the hang, since a calmly +// breathing "Working" on a wedged agent is the failure that actually costs you. +// This is an open number that wants tuning against live sessions. +const hangTimeout = 10 * time.Minute + +// deriveStatus computes the display status as one of five states. It reads the +// raw activity state plus the PR buckets directly, never a pre-collapsed +// status, so the active-vs-PR precedence below resolves the right way. +// +// Evaluated top to bottom, first match wins: a hung-or-never-booted session is +// caught before it can read Working, and the whole silent case outranks "go +// review". signalCapable says whether this harness has an activity hook +// pipeline at all; only then can prolonged silence mean the pipeline is broken. // -// A session may own several PRs at once (independent or stacked). The PR-derived -// status is the worst-wins aggregate across its open PRs; stacked children whose -// parent is still open are exempt from the aggregation since they cannot merge -// until the parent does. Merged/closed PRs only matter once no open PR remains. +// A session may own several PRs at once (independent or stacked). prMoves folds +// them into two booleans: a clean PR with a real action waiting on you, and an +// unfinished PR the agent left undone. A blocked stacked child's readiness is +// suppressed (it cannot merge until its parent does), but its problems still +// count as unfinished work. func deriveStatus(rec domain.SessionRecord, prs []domain.PRFacts, now time.Time, signalCapable bool) domain.SessionStatus { - if rec.IsTerminated { - if anyMerged(prs) { - return domain.StatusMerged - } - return domain.StatusTerminated - } + hasClean, hasUnfinished := prMoves(openPRs(prs)) - if rec.Activity.State == domain.ActivityWaitingInput { + switch { + case rec.IsTerminated: // includes merged + return domain.StatusIdle + case rec.Activity.State == domain.ActivityWaitingInput: return domain.StatusNeedsInput - } - - open := openPRs(prs) - if len(open) > 0 { - return aggregatePRStatus(open) - } - if anyMerged(prs) { - return domain.StatusMerged - } - - if rec.Activity.State == domain.ActivityActive { + case stalled(rec, now, signalCapable, hasUnfinished): + return domain.StatusStalled + case rec.Activity.State == domain.ActivityActive: return domain.StatusWorking + case hasClean: + return domain.StatusReady + default: + return domain.StatusIdle } +} - // No hook callback has ever arrived for this spawn/restore even though the - // harness has a hook pipeline. The seeded LastActivityAt marks the launch, - // so once the grace passes the honest status is "no signal", not "idle". - if signalCapable && rec.FirstSignalAt.IsZero() && now.Sub(rec.Activity.LastActivityAt) > noSignalGrace { - return domain.StatusNoSignal +// stalled reports whether the agent will not finish on its own: it never booted, +// hung mid-run, or stopped leaving its PR undone. +func stalled(rec domain.SessionRecord, now time.Time, signalCapable, hasUnfinished bool) bool { + active := rec.Activity.State == domain.ActivityActive + silence := now.Sub(rec.Activity.LastActivityAt) + switch { + case signalCapable && rec.FirstSignalAt.IsZero() && silence > bootGrace: + return true // never booted + case active && silence > hangTimeout: + return true // hung mid-run + case !active && hasUnfinished: + return true // stopped, work undone + default: + return false } - return domain.StatusIdle } // openPRs returns the PRs that are neither merged nor closed, preserving order. @@ -68,98 +80,56 @@ func openPRs(prs []domain.PRFacts) []domain.PRFacts { return out } -func anyMerged(prs []domain.PRFacts) bool { - for _, p := range prs { - if p.Merged { - return true - } - } - return false -} - -// aggregatePRStatus is the worst-wins reduction over a session's open PRs. -// A stacked child blocked by an open parent cannot merge yet, so its readiness -// signals (mergeable/approved/review-pending/open) are not actionable for the -// session and are suppressed. Its problem signals are still actionable: failing -// CI, draft state, and requested-changes/unresolved-comments must stay visible -// so a broken child is not hidden behind the stack. If no PR contributes any -// signal (a degenerate stack with no visible root), it falls back to aggregating -// the raw status across all open PRs so the session never goes dark. -func aggregatePRStatus(open []domain.PRFacts) domain.SessionStatus { +// prMoves folds a session's open PRs into whose move it is. hasClean is true +// when some PR has a real action waiting on you; hasUnfinished is true when some +// PR is work the agent left undone. A blocked stacked child cannot merge until +// its parent does, so its clean signal is suppressed, but its unfinished signal +// still surfaces so a broken child is not hidden behind the stack. +// +// A pathological all-blocked-clean stack (a cycle of mergeable PRs each targeting +// the next's branch, with no root) yields neither move and reads Idle. The old +// model force-aggregated such a set to avoid "going dark"; the new model treats +// genuinely no-actionable-move PRs as Idle, which is honest, and the cycle cannot +// arise from a real branch topology. +func prMoves(open []domain.PRFacts) (hasClean, hasUnfinished bool) { stacks := buildStacks(open) - candidates := make([]domain.SessionStatus, 0, len(open)) for _, p := range open { - s := prPipelineStatus(p) - if stacks[p.URL].Blocked && !isActionableChildSignal(s) { - continue - } - candidates = append(candidates, s) - } - if len(candidates) == 0 { - for _, p := range open { - candidates = append(candidates, prPipelineStatus(p)) - } - } - worst := candidates[0] - for _, s := range candidates[1:] { - if statusSeverity(s) < statusSeverity(worst) { - worst = s + switch prBucket(p) { + case bucketUnfinished: + hasUnfinished = true + case bucketClean: + if !stacks[p.URL].Blocked { + hasClean = true + } } } - return worst + return hasClean, hasUnfinished } -// isActionableChildSignal reports whether a blocked stacked child's pipeline -// status is a problem the user can act on now, independent of the child's -// inability to merge until its parent does. -func isActionableChildSignal(s domain.SessionStatus) bool { - switch s { - case domain.StatusCIFailed, domain.StatusDraft, domain.StatusChangesRequested: - return true - default: - return false - } -} +type prBucketKind int -// statusSeverity ranks PR pipeline statuses from most to least urgent so the -// aggregate surfaces the PR that most needs attention. mergeable is least urgent -// so a session only reports mergeable when every aggregated PR is mergeable. -func statusSeverity(s domain.SessionStatus) int { - switch s { - case domain.StatusCIFailed: - return 0 - case domain.StatusChangesRequested: - return 1 - case domain.StatusDraft: - return 2 - case domain.StatusReviewPending: - return 3 - case domain.StatusPROpen: - return 4 - case domain.StatusApproved: - return 5 - case domain.StatusMergeable: - return 6 - default: - return 7 - } -} +const ( + bucketNeutral prBucketKind = iota // bare open PR, just sitting there + bucketClean // a clean action waits on you + bucketUnfinished // the agent has more to do +) -func prPipelineStatus(pr domain.PRFacts) domain.SessionStatus { +// prBucket sorts one PR by whose move it is. Unfinished signals (failing CI, +// draft, requested changes, unresolved comments, merge conflict) are checked +// first so a PR that is both broken and otherwise mergeable reads as unfinished. +func prBucket(pr domain.PRFacts) prBucketKind { 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 + case pr.CI == domain.CIFailing, + pr.Draft, + pr.Review == domain.ReviewChangesRequest, + pr.ReviewComments, + pr.Mergeability == domain.MergeConflicting: + return bucketUnfinished + case pr.Mergeability == domain.MergeMergeable, + pr.Review == domain.ReviewApproved, + pr.Review == domain.ReviewRequired: + return bucketClean default: - return domain.StatusPROpen + return bucketNeutral } } diff --git a/backend/internal/service/session/status_test.go b/backend/internal/service/session/status_test.go index f989ed49..8de63b77 100644 --- a/backend/internal/service/session/status_test.go +++ b/backend/internal/service/session/status_test.go @@ -35,40 +35,57 @@ func TestServiceDerivesStatusFromSessionFactsAndPR(t *testing.T) { rec domain.SessionRecord pr []domain.PRFacts // hookless marks a harness with no activity pipeline (signalCapable - // false): silence is its permanent normal state, never no_signal. + // false): silence is its permanent normal state, never stalled. hookless bool want domain.SessionStatus }{ - {"terminated", statusRec(domain.ActivityExited, true), nil, false, domain.StatusTerminated}, - {"merged-pr", statusRec(domain.ActivityIdle, true), statusPR(domain.PRFacts{Merged: true}), false, domain.StatusMerged}, + // Terminated and merged both collapse to Idle. + {"terminated", statusRec(domain.ActivityExited, true), nil, false, domain.StatusIdle}, + {"merged-pr", statusRec(domain.ActivityIdle, true), statusPR(domain.PRFacts{Merged: true}), false, domain.StatusIdle}, + + // waiting_input outranks every PR fact. {"needs-input", statusRec(domain.ActivityWaitingInput, false), statusPR(domain.PRFacts{CI: domain.CIFailing}), false, domain.StatusNeedsInput}, - {"ci-failed", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{CI: domain.CIFailing}), false, domain.StatusCIFailed}, - {"draft", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Draft: true}), false, domain.StatusDraft}, - {"changes-requested", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewChangesRequest}), false, domain.StatusChangesRequested}, - {"mergeable", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Mergeability: domain.MergeMergeable}), false, domain.StatusMergeable}, - {"approved", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewApproved}), false, domain.StatusApproved}, - {"review-pending", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewRequired}), false, domain.StatusReviewPending}, - {"pr-open", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{}), false, domain.StatusPROpen}, + + // Stopped on an unfinished PR is Stalled, not Ready: the agent had the + // move and quit. + {"stopped-ci-failed", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{CI: domain.CIFailing}), false, domain.StatusStalled}, + {"stopped-draft", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Draft: true}), false, domain.StatusStalled}, + {"stopped-changes-requested", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewChangesRequest}), false, domain.StatusStalled}, + {"stopped-conflicting", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Mergeability: domain.MergeConflicting}), false, domain.StatusStalled}, + + // An active agent on top of any PR keeps Working (active-wins). + {"active-on-unfinished-pr", statusRec(domain.ActivityActive, false), statusPR(domain.PRFacts{CI: domain.CIFailing}), false, domain.StatusWorking}, + {"active-on-clean-pr", statusRec(domain.ActivityActive, false), statusPR(domain.PRFacts{Mergeability: domain.MergeMergeable}), false, domain.StatusWorking}, + + // Stopped on a clean PR is Ready. + {"stopped-mergeable", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Mergeability: domain.MergeMergeable}), false, domain.StatusReady}, + {"stopped-approved", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewApproved}), false, domain.StatusReady}, + {"stopped-review-required", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{Review: domain.ReviewRequired}), false, domain.StatusReady}, + + // Bare open PR behaves as no PR: stopped reads Idle. + {"stopped-bare-pr-open", statusRec(domain.ActivityIdle, false), statusPR(domain.PRFacts{}), false, domain.StatusIdle}, + {"working", statusRec(domain.ActivityActive, false), nil, false, domain.StatusWorking}, {"idle", statusRec(domain.ActivityIdle, false), nil, false, domain.StatusIdle}, - // A live session whose hook-capable agent never signaled is no_signal - // once the grace passes — never a confident idle. - {"no-signal-after-grace", silentRec(2 * noSignalGrace), nil, false, domain.StatusNoSignal}, - // A hook-less harness can never signal: its silence stays idle forever - // instead of degrading into a false "needs you". - {"hookless-silent-stays-idle", silentRec(2 * noSignalGrace), nil, true, domain.StatusIdle}, + // A live session whose hook-capable agent never signaled is Stalled + // once the boot grace passes — never a confident idle. + {"never-booted-after-grace", silentRec(2 * bootGrace), nil, false, domain.StatusStalled}, + // A hook-less harness can never signal: its silence stays idle forever. + {"hookless-silent-stays-idle", silentRec(2 * bootGrace), nil, true, domain.StatusIdle}, // Right after spawn the agent legitimately hasn't called back yet. {"silent-within-grace-is-idle", silentRec(10 * time.Second), nil, false, domain.StatusIdle}, - // Termination and PR facts outrank the missing-signal downgrade. + + // Termination outranks the never-booted downgrade. { - "no-signal-terminated-wins", - domain.SessionRecord{Activity: domain.Activity{State: domain.ActivityExited, LastActivityAt: statusNow.Add(-2 * noSignalGrace)}, IsTerminated: true}, + "terminated-outranks-never-booted", + domain.SessionRecord{Activity: domain.Activity{State: domain.ActivityExited, LastActivityAt: statusNow.Add(-2 * bootGrace)}, IsTerminated: true}, nil, false, - domain.StatusTerminated, + domain.StatusIdle, }, - {"no-signal-pr-wins", silentRec(2 * noSignalGrace), statusPR(domain.PRFacts{}), false, domain.StatusPROpen}, + // Never-booted silence outranks a bare open PR (neutral → idle anyway). + {"never-booted-bare-pr", silentRec(2 * bootGrace), statusPR(domain.PRFacts{}), false, domain.StatusStalled}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -79,10 +96,28 @@ func TestServiceDerivesStatusFromSessionFactsAndPR(t *testing.T) { } } +// An active session gone silent past hangTimeout is caught as Stalled instead +// of reading a calm Working; within the timeout it stays Working. +func TestDeriveStatusHungActiveSessionStalls(t *testing.T) { + hung := domain.SessionRecord{ + Activity: domain.Activity{State: domain.ActivityActive, LastActivityAt: statusNow.Add(-2 * hangTimeout)}, + FirstSignalAt: statusNow.Add(-3 * hangTimeout), + } + if got := deriveStatus(hung, nil, statusNow, true); got != domain.StatusStalled { + t.Fatalf("got %q want stalled", got) + } + live := domain.SessionRecord{ + Activity: domain.Activity{State: domain.ActivityActive, LastActivityAt: statusNow.Add(-1 * time.Minute)}, + FirstSignalAt: statusNow.Add(-1 * time.Hour), + } + if got := deriveStatus(live, nil, statusNow, true); got != domain.StatusWorking { + t.Fatalf("got %q want working", got) + } +} + // A blocked stacked child cannot merge until its parent does, so its readiness -// signals are suppressed, but its problem signals (failing CI, draft, -// requested-changes/unresolved-comments) must still surface for the session. -func TestAggregateStackedChildSignals(t *testing.T) { +// is suppressed, but its problem signals still surface as unfinished work. +func TestStackedChildSignals(t *testing.T) { parent := domain.PRFacts{URL: "parent", SourceBranch: "feat", Mergeability: domain.MergeMergeable} child := func(f domain.PRFacts) domain.PRFacts { f.URL = "child" @@ -95,24 +130,15 @@ func TestAggregateStackedChildSignals(t *testing.T) { prs []domain.PRFacts want domain.SessionStatus }{ - {"blocked-child-ci-failing-surfaces", []domain.PRFacts{parent, child(domain.PRFacts{CI: domain.CIFailing})}, domain.StatusCIFailed}, - {"blocked-child-draft-surfaces", []domain.PRFacts{parent, child(domain.PRFacts{Draft: true})}, domain.StatusDraft}, - {"blocked-child-changes-requested-surfaces", []domain.PRFacts{parent, child(domain.PRFacts{Review: domain.ReviewChangesRequest})}, domain.StatusChangesRequested}, - {"blocked-child-unresolved-comments-surfaces", []domain.PRFacts{parent, child(domain.PRFacts{ReviewComments: true})}, domain.StatusChangesRequested}, - // A blocked child's readiness signals stay hidden: only the parent's - // mergeable state drives the session. - {"blocked-child-mergeable-suppressed", []domain.PRFacts{parent, child(domain.PRFacts{Mergeability: domain.MergeMergeable})}, domain.StatusMergeable}, - {"blocked-child-approved-suppressed", []domain.PRFacts{parent, child(domain.PRFacts{Review: domain.ReviewApproved})}, domain.StatusMergeable}, - // Degenerate set where every open PR is blocked and none is actionable: - // fall back to the raw aggregate so the session never goes dark. - { - "all-blocked-no-actionable-falls-back", - []domain.PRFacts{ - {URL: "a", SourceBranch: "feat/a", TargetBranch: "feat/b", Mergeability: domain.MergeMergeable}, - {URL: "b", SourceBranch: "feat/b", TargetBranch: "feat/a", Mergeability: domain.MergeMergeable}, - }, - domain.StatusMergeable, - }, + // A blocked child's problem drags the stopped session to Stalled. + {"blocked-child-ci-failing-stalls", []domain.PRFacts{parent, child(domain.PRFacts{CI: domain.CIFailing})}, domain.StatusStalled}, + {"blocked-child-draft-stalls", []domain.PRFacts{parent, child(domain.PRFacts{Draft: true})}, domain.StatusStalled}, + {"blocked-child-changes-requested-stalls", []domain.PRFacts{parent, child(domain.PRFacts{Review: domain.ReviewChangesRequest})}, domain.StatusStalled}, + {"blocked-child-unresolved-comments-stalls", []domain.PRFacts{parent, child(domain.PRFacts{ReviewComments: true})}, domain.StatusStalled}, + // A blocked child's readiness stays hidden: the parent's clean state + // alone drives the session to Ready. + {"blocked-child-mergeable-suppressed", []domain.PRFacts{parent, child(domain.PRFacts{Mergeability: domain.MergeMergeable})}, domain.StatusReady}, + {"blocked-child-approved-suppressed", []domain.PRFacts{parent, child(domain.PRFacts{Review: domain.ReviewApproved})}, domain.StatusReady}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -123,11 +149,11 @@ func TestAggregateStackedChildSignals(t *testing.T) { } } -// Without an injected capability predicate the service must never claim -// no_signal; with one, capability follows the predicate per harness. +// Without an injected capability predicate the service must never claim a +// signal-driven stall; with one, capability follows the predicate per harness. func TestHarnessSignalsCapabilityGate(t *testing.T) { if (&Service{}).harnessSignals(domain.HarnessCodex) { - t.Fatal("zero-value Service reports signal-capable; want incapable (never no_signal)") + t.Fatal("zero-value Service reports signal-capable; want incapable (never stalled on silence)") } s := NewWithDeps(Deps{SignalCapable: func(h domain.AgentHarness) bool { return h == domain.HarnessCodex }}) if !s.harnessSignals(domain.HarnessCodex) { diff --git a/frontend/src/renderer/components/PullRequestsPage.test.tsx b/frontend/src/renderer/components/PullRequestsPage.test.tsx index 90a770bb..a0fc2d0a 100644 --- a/frontend/src/renderer/components/PullRequestsPage.test.tsx +++ b/frontend/src/renderer/components/PullRequestsPage.test.tsx @@ -40,7 +40,7 @@ const session = (id: string, prs: PullRequestFacts[]): WorkspaceSession => ({ provider: "claude-code", kind: "worker", branch: "feat/ns", - status: "review_pending", + status: "ready", updatedAt: "2026-06-15T00:00:00Z", prs, }); diff --git a/frontend/src/renderer/components/SessionInspector.test.tsx b/frontend/src/renderer/components/SessionInspector.test.tsx index bb517969..3a9c82c8 100644 --- a/frontend/src/renderer/components/SessionInspector.test.tsx +++ b/frontend/src/renderer/components/SessionInspector.test.tsx @@ -44,7 +44,7 @@ const session = (prs: PullRequestFacts[]): WorkspaceSession => ({ provider: "claude-code", kind: "worker", branch: "feat/ns", - status: "review_pending", + status: "ready", updatedAt: "2026-06-15T00:00:00Z", prs, }); diff --git a/frontend/src/renderer/components/SessionInspector.tsx b/frontend/src/renderer/components/SessionInspector.tsx index 87edce1d..6fac93ae 100644 --- a/frontend/src/renderer/components/SessionInspector.tsx +++ b/frontend/src/renderer/components/SessionInspector.tsx @@ -6,7 +6,7 @@ import { apiClient, apiErrorMessage } from "../lib/api-client"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { formatTimeCompact } from "../lib/format-time"; import type { PRState, PullRequestFacts, SessionStatus, WorkspaceSession } from "../types/workspace"; -import { sortedPRs, workerDisplayStatus } from "../types/workspace"; +import { sortedPRs, STATUS_META } from "../types/workspace"; import { Badge } from "./ui/badge"; import { cn } from "../lib/utils"; @@ -247,28 +247,17 @@ function activityDetail(status: SessionStatus): string | null { return "Session idle"; case "needs_input": return "Waiting for your input"; - case "working": - return null; + case "ready": + return "Clean PR waiting on you"; + case "stalled": + return "Not making progress; get it moving"; default: return null; } } -const STATUS_PILL: Record< - ReturnType | "idle", - { label: string; tone: string; breathe: boolean } -> = { - working: { label: "Working", tone: "var(--orange)", breathe: true }, - needs_you: { label: "Input needed", tone: "var(--amber)", breathe: false }, - ci_failed: { label: "CI failed", tone: "var(--red)", breathe: false }, - mergeable: { label: "Ready", tone: "var(--green)", breathe: false }, - done: { label: "Done", tone: "var(--fg-muted)", breathe: false }, - idle: { label: "Idle", tone: "var(--fg-muted)", breathe: false }, -}; - function InspectorStatusPill({ session }: { session: WorkspaceSession }) { - const key = session.status === "idle" ? "idle" : workerDisplayStatus(session); - const { label, tone, breathe } = STATUS_PILL[key]; + const { label, tone, breathe } = STATUS_META[session.status]; return ( ({ + status, + label: STATUS_META[status].label, + tone: STATUS_META[status].tone, +})); export function SessionsBoard({ projectId }: SessionsBoardProps) { const navigate = useNavigate(); @@ -69,17 +41,22 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { const workspaces = projectId ? all.filter((w) => w.id === projectId) : all; const sessions = workspaces.flatMap((w) => workerSessions(w.sessions)); const orchestrator = projectId - ? workspaces[0]?.sessions.find((session) => session.kind === "orchestrator" && session.status !== "terminated") + ? workspaces[0]?.sessions.find((session) => session.kind === "orchestrator" && !session.isTerminated) : undefined; const [isNewTaskOpen, setIsNewTaskOpen] = useState(false); const [isSpawning, setIsSpawning] = useState(false); - const byZone = new Map(); + // Live sessions group into the five status columns; terminated ones archive + // into the done bar regardless of their (idle) display status. + const byStatus = new Map(); + const done: WorkspaceSession[] = []; for (const session of sessions) { - const zone = attentionZone(session); - (byZone.get(zone) ?? byZone.set(zone, []).get(zone)!).push(session); + if (session.isTerminated) { + done.push(session); + continue; + } + (byStatus.get(session.status) ?? byStatus.set(session.status, []).get(session.status)!).push(session); } - const done = byZone.get("done") ?? []; // Collapsed by default, like agent-orchestrator's done-bar: finished and // killed sessions cost one quiet line under the board until expanded. const [doneExpanded, setDoneExpanded] = useState(false); @@ -157,9 +134,9 @@ export function SessionsBoard({ projectId }: SessionsBoardProps) { {workspaceQuery.isError ? (

Could not load sessions.

) : ( -
+
{COLUMNS.map((col) => ( - + ))}
)} @@ -228,20 +205,19 @@ function ZoneColumn({ sessions: WorkspaceSession[]; onOpen: (s: WorkspaceSession) => void; }) { + const glow = col.status === "idle" ? undefined : `0 0 7px color-mix(in srgb, ${col.tone} 60%, transparent)`; return (
- - {col.label} + + + {col.label} + {sessions.length}
@@ -279,8 +255,8 @@ function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: ( type="button" >
- - + + {badge.label} @@ -324,29 +300,7 @@ function agentLabel(provider: WorkspaceSession["provider"]): string { } } -function sessionBadge(session: WorkspaceSession): { label: string; className: string } { - switch (session.status) { - case "needs_input": - return { label: "Input needed", className: "text-warning" }; - case "ci_failed": - return { label: "CI failed", className: "text-error" }; - case "changes_requested": - return { label: "Changes requested", className: "text-warning" }; - case "review_pending": - return { label: "Review pending", className: "text-muted-foreground" }; - case "draft": - return { label: "Draft PR", className: "text-muted-foreground" }; - case "pr_open": - return { label: "PR open", className: "text-muted-foreground" }; - case "approved": - return { label: "Approved", className: "text-success" }; - case "mergeable": - return { label: "Ready", className: "text-success" }; - case "merged": - return { label: "Merged", className: "text-passive" }; - case "terminated": - return { label: "Terminated", className: "text-passive" }; - default: - return { label: "Working", className: "text-working" }; - } +function sessionBadge(session: WorkspaceSession): { label: string; tone: string } { + const meta = STATUS_META[session.status]; + return { label: meta.label, tone: meta.tone }; } diff --git a/frontend/src/renderer/components/ShellTopbar.tsx b/frontend/src/renderer/components/ShellTopbar.tsx index ff8285de..04adbe48 100644 --- a/frontend/src/renderer/components/ShellTopbar.tsx +++ b/frontend/src/renderer/components/ShellTopbar.tsx @@ -6,8 +6,7 @@ import { findProjectOrchestrator, isOrchestratorSession, sessionIsActive, - workerDisplayStatus, - type WorkerDisplayStatus, + STATUS_META, type WorkspaceSession, } from "../types/workspace"; import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; @@ -23,17 +22,6 @@ const isMac = typeof navigator !== "undefined" && /Mac|iPod|iPhone|iPad/.test(na const dragStyle = isMac ? ({ WebkitAppRegion: "drag" } as React.CSSProperties) : undefined; const noDragStyle = isMac ? ({ WebkitAppRegion: "no-drag" } as React.CSSProperties) : undefined; -// Session status → pill tone, mirroring agent-orchestrator's StatusBadge -// (working=orange & breathing, input=amber, fail=red, ready=green, done=neutral). -// Tones are theme vars so the pill tracks the light/dark status palettes. -const STATUS_PILL: Record = { - working: { label: "Working", tone: "var(--orange)", breathe: true }, - needs_you: { label: "Needs input", tone: "var(--amber)", breathe: false }, - ci_failed: { label: "CI failed", tone: "var(--red)", breathe: false }, - mergeable: { label: "Ready", tone: "var(--green)", breathe: false }, - done: { label: "Done", tone: "var(--fg-muted)", breathe: false }, -}; - // The one app topbar (.dashboard-app-header), rendered by the shell layout // across the full window width — above both the sidebar and the route outlet — // so the crumb and actions sit at identical offsets on every screen and the @@ -313,7 +301,7 @@ export function TopbarKillButton({ session }: { session: WorkspaceSession }) { // StatusBadge --pill: tinted bordered pill (inset 25%-tone hairline + 7%-tone // fill) with a 6px dot that breathes while the agent is working. function SessionStatusPill({ session }: { session: WorkspaceSession }) { - const { label, tone, breathe } = STATUS_PILL[workerDisplayStatus(session)]; + const { label, tone, breathe } = STATUS_META[session.status]; return (