diff --git a/backend/internal/adapters/agent/codex/codex.go b/backend/internal/adapters/agent/codex/codex.go index d8e45d86..43d1f6b0 100644 --- a/backend/internal/adapters/agent/codex/codex.go +++ b/backend/internal/adapters/agent/codex/codex.go @@ -303,7 +303,10 @@ func appendTerminalCompatibilityFlags(cmd *[]string) { func appendApprovalFlags(cmd *[]string, permissions ports.PermissionMode) { switch normalizePermissionMode(permissions) { case ports.PermissionModeDefault: - // No flag: defer to the user's Codex config/default behavior. + // Codex sessions are AO-managed and run headlessly inside a terminal + // mux pane; default to no approval prompts unless project settings + // explicitly choose a more restrictive mode. + *cmd = append(*cmd, "--dangerously-bypass-approvals-and-sandbox") case ports.PermissionModeAcceptEdits: *cmd = append(*cmd, "--ask-for-approval", "on-request") case ports.PermissionModeAuto: diff --git a/backend/internal/adapters/agent/codex/codex_test.go b/backend/internal/adapters/agent/codex/codex_test.go index c19d5a9d..0981b251 100644 --- a/backend/internal/adapters/agent/codex/codex_test.go +++ b/backend/internal/adapters/agent/codex/codex_test.go @@ -98,19 +98,21 @@ func TestGetLaunchCommandMapsApprovalModes(t *testing.T) { notExpected string }{ { - name: "default", - permission: ports.PermissionModeDefault, - notExpected: "--ask-for-approval", + name: "default", + permission: ports.PermissionModeDefault, + want: []string{"--dangerously-bypass-approvals-and-sandbox"}, }, { - name: "accept-edits", - permission: ports.PermissionModeAcceptEdits, - want: []string{"--ask-for-approval", "on-request"}, + name: "accept-edits", + permission: ports.PermissionModeAcceptEdits, + want: []string{"--ask-for-approval", "on-request"}, + notExpected: "--dangerously-bypass-approvals-and-sandbox", }, { - name: "auto", - permission: ports.PermissionModeAuto, - want: []string{"--ask-for-approval", "on-request", "-c", `approvals_reviewer="auto_review"`}, + name: "auto", + permission: ports.PermissionModeAuto, + want: []string{"--ask-for-approval", "on-request", "-c", `approvals_reviewer="auto_review"`}, + notExpected: "--dangerously-bypass-approvals-and-sandbox", }, { name: "bypass-permissions", @@ -118,9 +120,9 @@ func TestGetLaunchCommandMapsApprovalModes(t *testing.T) { want: []string{"--dangerously-bypass-approvals-and-sandbox"}, }, { - name: "empty", - permission: "", - notExpected: "--ask-for-approval", + name: "empty", + permission: "", + want: []string{"--dangerously-bypass-approvals-and-sandbox"}, }, } diff --git a/backend/internal/cli/dto_drift_e2e_test.go b/backend/internal/cli/dto_drift_e2e_test.go index 8b3a91ef..46b7c226 100644 --- a/backend/internal/cli/dto_drift_e2e_test.go +++ b/backend/internal/cli/dto_drift_e2e_test.go @@ -94,7 +94,7 @@ func (f *fakeSessionService) Send(context.Context, domain.SessionID, string) err return nil } -func (f *fakeSessionService) ListPRs(context.Context, domain.SessionID) ([]domain.PRFacts, error) { +func (f *fakeSessionService) ListPRSummaries(context.Context, domain.SessionID) ([]sessionsvc.PRSummary, error) { return nil, nil } diff --git a/backend/internal/domain/agentconfig.go b/backend/internal/domain/agentconfig.go index 19fe10a0..fb4ba39f 100644 --- a/backend/internal/domain/agentconfig.go +++ b/backend/internal/domain/agentconfig.go @@ -10,9 +10,9 @@ type PermissionMode string // The permission modes adapters map onto their agent's native approval flags. const ( - // PermissionModeDefault is special: adapters emit no flag for it so the - // agent resolves its starting mode from the user's own config (e.g. - // Claude's TUI reading ~/.claude/settings.json defaultMode). + // PermissionModeDefault is special: adapters choose their own baseline + // behavior for it. Most defer to the agent's own config; some managed + // adapters may map it to a safer non-interactive default. PermissionModeDefault PermissionMode = "default" PermissionModeAcceptEdits PermissionMode = "accept-edits" PermissionModeAuto PermissionMode = "auto" @@ -25,8 +25,8 @@ const ( type AgentConfig struct { // Model overrides the agent's default model (e.g. claude-opus-4-5). Model string `json:"model,omitempty"` - // Permissions sets the agent's starting permission mode. Empty defers to - // the agent's own configuration. + // Permissions sets the agent's starting permission mode. Empty is treated + // like the adapter's default mode. Permissions PermissionMode `json:"permissions,omitempty"` } diff --git a/backend/internal/domain/session.go b/backend/internal/domain/session.go index 6cc639e5..19d777ac 100644 --- a/backend/internal/domain/session.go +++ b/backend/internal/domain/session.go @@ -59,7 +59,7 @@ type SessionRecord struct { // plus the derived display Status. type Session struct { SessionRecord - Status SessionStatus `json:"status"` + Status SessionStatus `json:"status" enum:"working,pr_open,draft,ci_failed,review_pending,changes_requested,approved,mergeable,merged,needs_input,idle,terminated,no_signal"` TerminalHandleID string `json:"terminalHandleId,omitempty"` // PRs are the session's attributed pull requests (one session can own many). // They feed status derivation and are surfaced on the API read model. Not diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index 72eaf6da..565ffc86 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -1285,6 +1285,20 @@ components: $ref: '#/components/schemas/SessionPRFacts' type: array status: + enum: + - working + - pr_open + - draft + - ci_failed + - review_pending + - changes_requested + - approved + - mergeable + - merged + - needs_input + - idle + - terminated + - no_signal type: string terminalHandleId: type: string @@ -1385,7 +1399,7 @@ components: properties: prs: items: - $ref: '#/components/schemas/SessionPRFacts' + $ref: '#/components/schemas/SessionPRSummary' type: array sessionId: type: string @@ -1732,15 +1746,57 @@ components: - sessionId - message type: object + SessionPRCISummary: + properties: + failingChecks: + items: + $ref: '#/components/schemas/SessionPRFailingCheck' + type: array + state: + enum: + - unknown + - pending + - passing + - failing + type: string + required: + - state + - failingChecks + type: object + SessionPRConflictFile: + properties: + path: + type: string + url: + type: string + required: + - path + type: object SessionPRFacts: properties: ci: + enum: + - unknown + - pending + - passing + - failing type: string mergeability: + enum: + - unknown + - mergeable + - conflicting + - blocked + - unstable type: string number: type: integer review: + enum: + - none + - approved + - changes_requested + - review_required type: string reviewComments: type: boolean @@ -1766,6 +1822,167 @@ components: - reviewComments - updatedAt type: object + SessionPRFailingCheck: + properties: + conclusion: + type: string + name: + type: string + status: + enum: + - failed + - cancelled + type: string + url: + type: string + required: + - name + - status + - conclusion + type: object + SessionPRMergeabilitySummary: + properties: + conflictFiles: + items: + $ref: '#/components/schemas/SessionPRConflictFile' + type: array + prUrl: + type: string + reasons: + items: + type: string + type: array + state: + enum: + - unknown + - mergeable + - conflicting + - blocked + - unstable + type: string + required: + - state + - reasons + - prUrl + type: object + SessionPRReviewCommentLink: + properties: + file: + type: string + line: + type: integer + url: + type: string + type: object + SessionPRReviewSummary: + properties: + decision: + enum: + - none + - approved + - changes_requested + - review_required + type: string + hasUnresolvedHumanComments: + type: boolean + unresolvedBy: + items: + $ref: '#/components/schemas/SessionPRUnresolvedReviewer' + type: array + required: + - decision + - hasUnresolvedHumanComments + - unresolvedBy + type: object + SessionPRSummary: + properties: + additions: + type: integer + author: + type: string + changedFiles: + type: integer + ci: + $ref: '#/components/schemas/SessionPRCISummary' + ciObservedAt: + format: date-time + type: string + deletions: + type: integer + headSha: + type: string + htmlUrl: + type: string + mergeability: + $ref: '#/components/schemas/SessionPRMergeabilitySummary' + number: + type: integer + observedAt: + format: date-time + type: string + provider: + enum: + - github + type: string + repo: + type: string + review: + $ref: '#/components/schemas/SessionPRReviewSummary' + reviewObservedAt: + format: date-time + type: string + sourceBranch: + type: string + state: + enum: + - draft + - open + - merged + - closed + type: string + targetBranch: + type: string + title: + type: string + updatedAt: + format: date-time + type: string + url: + type: string + required: + - url + - number + - title + - state + - provider + - repo + - author + - sourceBranch + - targetBranch + - headSha + - additions + - deletions + - changedFiles + - ci + - review + - mergeability + - updatedAt + type: object + SessionPRUnresolvedReviewer: + properties: + count: + type: integer + links: + items: + $ref: '#/components/schemas/SessionPRReviewCommentLink' + type: array + reviewerId: + type: string + required: + - reviewerId + - count + - links + type: object SessionResponse: properties: session: diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index 2aeca734..ddc2c24e 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -130,38 +130,46 @@ var schemaNames = map[string]string{ "DomainAgentConfig": "AgentConfig", "DomainRoleOverride": "RoleOverride", // httpd/controllers (wire envelopes) - "ControllersListProjectsResponse": "ListProjectsResponse", - "ControllersProjectResponse": "ProjectResponse", - "ControllersGetProjectResponse": "ProjectGetResponse", - "ControllersProjectOrDegraded": "ProjectOrDegraded", - "ControllersListSessionsQuery": "ListSessionsQuery", - "ControllersCleanupSessionsQuery": "CleanupSessionsQuery", - "ControllersListSessionsResponse": "ListSessionsResponse", - "ControllersSpawnSessionRequest": "SpawnSessionRequest", - "ControllersSessionResponse": "SessionResponse", - "ControllersRenameSessionRequest": "RenameSessionRequest", - "ControllersRenameSessionResponse": "RenameSessionResponse", - "ControllersRestoreSessionResponse": "RestoreSessionResponse", - "ControllersCleanupSessionsResponse": "CleanupSessionsResponse", - "ControllersCleanupSkippedSession": "CleanupSkippedSession", - "ControllersKillSessionResponse": "KillSessionResponse", - "ControllersRollbackSessionResponse": "RollbackSessionResponse", - "ControllersSendSessionMessageRequest": "SendSessionMessageRequest", - "ControllersSendSessionMessageResponse": "SendSessionMessageResponse", - "ControllersClaimPRResponse": "ClaimPRResponse", - "ControllersClaimPRRequest": "ClaimPRRequest", - "ControllersSessionPRFacts": "SessionPRFacts", - "ControllersListSessionPRsResponse": "ListSessionPRsResponse", - "ControllersSetActivityRequest": "SetActivityRequest", - "ControllersSetActivityResponse": "SetActivityResponse", - "ControllersSpawnOrchestratorRequest": "SpawnOrchestratorRequest", - "ControllersSpawnOrchestratorResponse": "SpawnOrchestratorResponse", - "ControllersOrchestratorResponse": "OrchestratorResponse", - "ControllersListNotificationsQuery": "ListNotificationsQuery", - "ControllersNotificationStreamQuery": "NotificationStreamQuery", - "ControllersNotificationTarget": "NotificationTarget", - "ControllersNotificationResponse": "NotificationResponse", - "ControllersListNotificationsResponse": "ListNotificationsResponse", + "ControllersListProjectsResponse": "ListProjectsResponse", + "ControllersProjectResponse": "ProjectResponse", + "ControllersGetProjectResponse": "ProjectGetResponse", + "ControllersProjectOrDegraded": "ProjectOrDegraded", + "ControllersListSessionsQuery": "ListSessionsQuery", + "ControllersCleanupSessionsQuery": "CleanupSessionsQuery", + "ControllersListSessionsResponse": "ListSessionsResponse", + "ControllersSpawnSessionRequest": "SpawnSessionRequest", + "ControllersSessionResponse": "SessionResponse", + "ControllersRenameSessionRequest": "RenameSessionRequest", + "ControllersRenameSessionResponse": "RenameSessionResponse", + "ControllersRestoreSessionResponse": "RestoreSessionResponse", + "ControllersCleanupSessionsResponse": "CleanupSessionsResponse", + "ControllersCleanupSkippedSession": "CleanupSkippedSession", + "ControllersKillSessionResponse": "KillSessionResponse", + "ControllersRollbackSessionResponse": "RollbackSessionResponse", + "ControllersSendSessionMessageRequest": "SendSessionMessageRequest", + "ControllersSendSessionMessageResponse": "SendSessionMessageResponse", + "ControllersClaimPRResponse": "ClaimPRResponse", + "ControllersClaimPRRequest": "ClaimPRRequest", + "ControllersSessionPRFacts": "SessionPRFacts", + "ControllersSessionPRSummary": "SessionPRSummary", + "ControllersSessionPRCISummary": "SessionPRCISummary", + "ControllersSessionPRFailingCheck": "SessionPRFailingCheck", + "ControllersSessionPRReviewSummary": "SessionPRReviewSummary", + "ControllersSessionPRUnresolvedReviewer": "SessionPRUnresolvedReviewer", + "ControllersSessionPRReviewCommentLink": "SessionPRReviewCommentLink", + "ControllersSessionPRMergeabilitySummary": "SessionPRMergeabilitySummary", + "ControllersSessionPRConflictFile": "SessionPRConflictFile", + "ControllersListSessionPRsResponse": "ListSessionPRsResponse", + "ControllersSetActivityRequest": "SetActivityRequest", + "ControllersSetActivityResponse": "SetActivityResponse", + "ControllersSpawnOrchestratorRequest": "SpawnOrchestratorRequest", + "ControllersSpawnOrchestratorResponse": "SpawnOrchestratorResponse", + "ControllersOrchestratorResponse": "OrchestratorResponse", + "ControllersListNotificationsQuery": "ListNotificationsQuery", + "ControllersNotificationStreamQuery": "NotificationStreamQuery", + "ControllersNotificationTarget": "NotificationTarget", + "ControllersNotificationResponse": "NotificationResponse", + "ControllersListNotificationsResponse": "ListNotificationsResponse", // httpd/controllers — PR wire envelopes "ControllersMergePRResponse": "MergePRResponse", "ControllersResolveCommentsRequest": "ResolveCommentsRequest", diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index 8a235a38..c893cecb 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -7,6 +7,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" + sessionsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/session" ) // HTTP response envelopes for the projects surface — the SINGLE definition of @@ -210,17 +211,148 @@ type SessionPRFacts struct { URL string `json:"url"` Number int `json:"number"` State string `json:"state" enum:"draft,open,merged,closed"` - CI domain.CIState `json:"ci"` - Review domain.ReviewDecision `json:"review"` - Mergeability domain.Mergeability `json:"mergeability"` + CI domain.CIState `json:"ci" enum:"unknown,pending,passing,failing"` + Review domain.ReviewDecision `json:"review" enum:"none,approved,changes_requested,review_required"` + Mergeability domain.Mergeability `json:"mergeability" enum:"unknown,mergeable,conflicting,blocked,unstable"` ReviewComments bool `json:"reviewComments"` UpdatedAt time.Time `json:"updatedAt"` } +// SessionPRSummary is the concise desktop SCM read model returned by GET +// /sessions/{sessionId}/pr. It intentionally omits CI log tails and review +// comment bodies. +type SessionPRSummary struct { + URL string `json:"url"` + HTMLURL string `json:"htmlUrl,omitempty"` + Number int `json:"number"` + Title string `json:"title"` + State domain.PRState `json:"state" enum:"draft,open,merged,closed"` + Provider string `json:"provider" enum:"github"` + Repo string `json:"repo"` + Author string `json:"author"` + SourceBranch string `json:"sourceBranch"` + TargetBranch string `json:"targetBranch"` + HeadSHA string `json:"headSha"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + ChangedFiles int `json:"changedFiles"` + CI SessionPRCISummary `json:"ci"` + Review SessionPRReviewSummary `json:"review"` + Mergeability SessionPRMergeabilitySummary `json:"mergeability"` + UpdatedAt time.Time `json:"updatedAt"` + ObservedAt time.Time `json:"observedAt,omitempty"` + CIObservedAt time.Time `json:"ciObservedAt,omitempty"` + ReviewObservedAt time.Time `json:"reviewObservedAt,omitempty"` +} + +// SessionPRCISummary is the CI status block for a session PR summary. +type SessionPRCISummary struct { + State domain.CIState `json:"state" enum:"unknown,pending,passing,failing"` + FailingChecks []SessionPRFailingCheck `json:"failingChecks"` +} + +// SessionPRFailingCheck is one failed or cancelled CI check for a PR. +type SessionPRFailingCheck struct { + Name string `json:"name"` + Status domain.PRCheckStatus `json:"status" enum:"failed,cancelled"` + Conclusion string `json:"conclusion"` + URL string `json:"url,omitempty"` +} + +// SessionPRReviewSummary is the review state block for a session PR summary. +type SessionPRReviewSummary struct { + Decision domain.ReviewDecision `json:"decision" enum:"none,approved,changes_requested,review_required"` + HasUnresolvedHumanComments bool `json:"hasUnresolvedHumanComments"` + UnresolvedBy []SessionPRUnresolvedReviewer `json:"unresolvedBy"` +} + +// SessionPRUnresolvedReviewer groups unresolved human comments by reviewer. +type SessionPRUnresolvedReviewer struct { + ReviewerID string `json:"reviewerId"` + Count int `json:"count"` + Links []SessionPRReviewCommentLink `json:"links"` +} + +// SessionPRReviewCommentLink points to one unresolved review comment. +type SessionPRReviewCommentLink struct { + URL string `json:"url,omitempty"` + File string `json:"file,omitempty"` + Line int `json:"line,omitempty"` +} + +// SessionPRMergeabilitySummary is the mergeability block for a session PR summary. +type SessionPRMergeabilitySummary struct { + State domain.Mergeability `json:"state" enum:"unknown,mergeable,conflicting,blocked,unstable"` + Reasons []string `json:"reasons"` + PRURL string `json:"prUrl"` + ConflictFiles []SessionPRConflictFile `json:"conflictFiles,omitempty"` +} + +// SessionPRConflictFile is one file involved in a PR merge conflict. +type SessionPRConflictFile struct { + Path string `json:"path"` + URL string `json:"url,omitempty"` +} + // ListSessionPRsResponse is the body of GET /sessions/{sessionId}/pr. type ListSessionPRsResponse struct { - SessionID domain.SessionID `json:"sessionId"` - PRs []SessionPRFacts `json:"prs"` + SessionID domain.SessionID `json:"sessionId"` + PRs []SessionPRSummary `json:"prs"` +} + +// NewSessionPRSummary maps the service PR summary model to its HTTP DTO. +func NewSessionPRSummary(in sessionsvc.PRSummary) SessionPRSummary { + return SessionPRSummary{ + URL: in.URL, + HTMLURL: in.HTMLURL, + Number: in.Number, + Title: in.Title, + State: in.State, + Provider: in.Provider, + Repo: in.Repo, + Author: in.Author, + SourceBranch: in.SourceBranch, + TargetBranch: in.TargetBranch, + HeadSHA: in.HeadSHA, + Additions: in.Additions, + Deletions: in.Deletions, + ChangedFiles: in.ChangedFiles, + CI: newSessionPRCISummary(in.CI), + Review: newSessionPRReviewSummary(in.Review), + Mergeability: newSessionPRMergeabilitySummary(in.Mergeability), + UpdatedAt: in.UpdatedAt, + ObservedAt: in.ObservedAt, + CIObservedAt: in.CIObservedAt, + ReviewObservedAt: in.ReviewObservedAt, + } +} + +func newSessionPRCISummary(in sessionsvc.PRCISummary) SessionPRCISummary { + checks := make([]SessionPRFailingCheck, 0, len(in.FailingChecks)) + for _, ch := range in.FailingChecks { + checks = append(checks, SessionPRFailingCheck{Name: ch.Name, Status: ch.Status, Conclusion: ch.Conclusion, URL: ch.URL}) + } + return SessionPRCISummary{State: in.State, FailingChecks: checks} +} + +func newSessionPRReviewSummary(in sessionsvc.PRReviewSummary) SessionPRReviewSummary { + reviewers := make([]SessionPRUnresolvedReviewer, 0, len(in.UnresolvedBy)) + for _, reviewer := range in.UnresolvedBy { + links := make([]SessionPRReviewCommentLink, 0, len(reviewer.Links)) + for _, link := range reviewer.Links { + links = append(links, SessionPRReviewCommentLink{URL: link.URL, File: link.File, Line: link.Line}) + } + reviewers = append(reviewers, SessionPRUnresolvedReviewer{ReviewerID: reviewer.ReviewerID, Count: reviewer.Count, Links: links}) + } + return SessionPRReviewSummary{Decision: in.Decision, HasUnresolvedHumanComments: in.HasUnresolvedHumanComments, UnresolvedBy: reviewers} +} + +func newSessionPRMergeabilitySummary(in sessionsvc.PRMergeabilitySummary) SessionPRMergeabilitySummary { + files := make([]SessionPRConflictFile, 0, len(in.ConflictFiles)) + for _, file := range in.ConflictFiles { + files = append(files, SessionPRConflictFile{Path: file.Path, URL: file.URL}) + } + return SessionPRMergeabilitySummary{State: in.State, Reasons: in.Reasons, PRURL: in.PRURL, ConflictFiles: files} } // ClaimPRRequest is the body of POST /sessions/{sessionId}/pr/claim. diff --git a/backend/internal/httpd/controllers/sessions.go b/backend/internal/httpd/controllers/sessions.go index e1473edd..80e63862 100644 --- a/backend/internal/httpd/controllers/sessions.go +++ b/backend/internal/httpd/controllers/sessions.go @@ -33,7 +33,7 @@ type SessionService interface { Cleanup(ctx context.Context, project domain.ProjectID) (sessionsvc.CleanupOutcome, error) Rename(ctx context.Context, id domain.SessionID, displayName string) error Send(ctx context.Context, id domain.SessionID, message string) error - ListPRs(ctx context.Context, id domain.SessionID) ([]domain.PRFacts, error) + ListPRSummaries(ctx context.Context, id domain.SessionID) ([]sessionsvc.PRSummary, error) ClaimPR(ctx context.Context, id domain.SessionID, ref string, opts sessionsvc.ClaimPROptions) (sessionsvc.ClaimPRResult, error) } @@ -137,12 +137,12 @@ func (c *SessionsController) listPRs(w http.ResponseWriter, r *http.Request) { apispec.NotImplemented(w, r, "GET", "/api/v1/sessions/{sessionId}/pr") return } - prs, err := c.Svc.ListPRs(r.Context(), sessionID(r)) + prs, err := c.Svc.ListPRSummaries(r.Context(), sessionID(r)) if err != nil { envelope.WriteError(w, r, err) return } - envelope.WriteJSON(w, http.StatusOK, ListSessionPRsResponse{SessionID: sessionID(r), PRs: sessionPRFacts(prs)}) + envelope.WriteJSON(w, http.StatusOK, ListSessionPRsResponse{SessionID: sessionID(r), PRs: sessionPRSummaries(prs)}) } func (c *SessionsController) claimPR(w http.ResponseWriter, r *http.Request) { @@ -446,6 +446,14 @@ func sessionPRFacts(prs []domain.PRFacts) []SessionPRFacts { return out } +func sessionPRSummaries(prs []sessionsvc.PRSummary) []SessionPRSummary { + out := make([]SessionPRSummary, 0, len(prs)) + for _, pr := range prs { + out = append(out, NewSessionPRSummary(pr)) + } + return out +} + func prState(pr domain.PRFacts) string { switch { case pr.Merged: diff --git a/backend/internal/httpd/controllers/sessions_test.go b/backend/internal/httpd/controllers/sessions_test.go index 177e8c7c..6a0ec96e 100644 --- a/backend/internal/httpd/controllers/sessions_test.go +++ b/backend/internal/httpd/controllers/sessions_test.go @@ -139,6 +139,49 @@ func (f *fakeSessionService) ListPRs(_ context.Context, id domain.SessionID) ([] return []domain.PRFacts{{URL: "https://github.com/aoagents/agent-orchestrator/pull/142", Number: 142, CI: domain.CIPassing, Review: domain.ReviewRequired, Mergeability: domain.MergeMergeable, UpdatedAt: time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC)}}, nil } +func (f *fakeSessionService) ListPRSummaries(_ context.Context, id domain.SessionID) ([]sessionsvc.PRSummary, error) { + if f.listPRErr != nil { + return nil, f.listPRErr + } + if _, ok := f.sessions[id]; !ok { + return nil, apierr.NotFound("SESSION_NOT_FOUND", "Unknown session") + } + return []sessionsvc.PRSummary{{ + URL: "https://github.com/aoagents/agent-orchestrator/pull/142", + HTMLURL: "https://github.com/aoagents/agent-orchestrator/pull/142", + Number: 142, + Title: "Wire SCM summaries", + State: domain.PRStateOpen, + Provider: "github", + Repo: "aoagents/agent-orchestrator", + Author: "ada", + SourceBranch: "codex/scm-observer-v1", + TargetBranch: "main", + HeadSHA: "abc123", + CI: sessionsvc.PRCISummary{State: domain.CIFailing, FailingChecks: []sessionsvc.PRFailingCheck{{ + Name: "unit", + Status: domain.PRCheckFailed, + Conclusion: "failure", + URL: "https://github.com/aoagents/agent-orchestrator/actions/runs/1", + }}}, + Review: sessionsvc.PRReviewSummary{ + Decision: domain.ReviewChangesRequest, + HasUnresolvedHumanComments: true, + UnresolvedBy: []sessionsvc.PRUnresolvedReviewer{{ + ReviewerID: "reviewer-a", + Count: 1, + Links: []sessionsvc.PRReviewCommentLink{{URL: "https://github.com/aoagents/agent-orchestrator/pull/142#discussion_r1", File: "main.go", Line: 12}}, + }}, + }, + Mergeability: sessionsvc.PRMergeabilitySummary{ + State: domain.MergeConflicting, + Reasons: []string{"conflicts"}, + PRURL: "https://github.com/aoagents/agent-orchestrator/pull/142", + }, + UpdatedAt: time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC), + }}, nil +} + func (f *fakeSessionService) ClaimPR(_ context.Context, id domain.SessionID, ref string, opts sessionsvc.ClaimPROptions) (sessionsvc.ClaimPRResult, error) { if f.claimErr != nil { return sessionsvc.ClaimPRResult{}, f.claimErr @@ -422,16 +465,56 @@ func TestSessionsAPI_PRRoutes(t *testing.T) { var listed struct { SessionID string `json:"sessionId"` PRs []struct { - URL string `json:"url"` - Number int `json:"number"` - State string `json:"state"` - UpdatedAt string `json:"updatedAt"` + URL string `json:"url"` + Number int `json:"number"` + Title string `json:"title"` + State string `json:"state"` + CI struct { + State string `json:"state"` + FailingChecks []struct { + Name string `json:"name"` + Status string `json:"status"` + Conclusion string `json:"conclusion"` + URL string `json:"url"` + LogTail string `json:"logTail"` + } `json:"failingChecks"` + } `json:"ci"` + Review struct { + Decision string `json:"decision"` + UnresolvedBy []struct { + ReviewerID string `json:"reviewerId"` + Count int `json:"count"` + Links []struct { + URL string `json:"url"` + File string `json:"file"` + Line int `json:"line"` + Body string `json:"body"` + } `json:"links"` + } `json:"unresolvedBy"` + } `json:"review"` + Mergeability struct { + State string `json:"state"` + Reasons []string `json:"reasons"` + PRURL string `json:"prUrl"` + ConflictFiles []struct { + Path string `json:"path"` + } `json:"conflictFiles"` + } `json:"mergeability"` } `json:"prs"` } mustJSON(t, body, &listed) - if listed.SessionID != "ao-1" || len(listed.PRs) != 1 || listed.PRs[0].State != "open" { + if listed.SessionID != "ao-1" || len(listed.PRs) != 1 || listed.PRs[0].State != "open" || listed.PRs[0].Title == "" { t.Fatalf("GET shape = %#v", listed) } + if checks := listed.PRs[0].CI.FailingChecks; len(checks) != 1 || checks[0].Name != "unit" || checks[0].LogTail != "" { + t.Fatalf("failing checks = %#v", checks) + } + if reviewers := listed.PRs[0].Review.UnresolvedBy; len(reviewers) != 1 || reviewers[0].ReviewerID != "reviewer-a" || reviewers[0].Links[0].Body != "" { + t.Fatalf("reviewers = %#v", reviewers) + } + if merge := listed.PRs[0].Mergeability; merge.State != "conflicting" || len(merge.ConflictFiles) != 0 || merge.PRURL == "" { + t.Fatalf("mergeability = %#v", merge) + } body, status, _ = doRequest(t, srv, "POST", "/api/v1/sessions/ao-1/pr/claim", `{"pr":"142"}`) if status != http.StatusOK { diff --git a/backend/internal/service/session/pr_summary.go b/backend/internal/service/session/pr_summary.go new file mode 100644 index 00000000..1a09fb5f --- /dev/null +++ b/backend/internal/service/session/pr_summary.go @@ -0,0 +1,316 @@ +package session + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apierr" +) + +// PRSummary is the user-facing SCM read model for one PR owned by a session. +type PRSummary struct { + URL string + HTMLURL string + Number int + Title string + State domain.PRState + Provider string + Repo string + Author string + SourceBranch string + TargetBranch string + HeadSHA string + Additions int + Deletions int + ChangedFiles int + CI PRCISummary + Review PRReviewSummary + Mergeability PRMergeabilitySummary + UpdatedAt time.Time + ObservedAt time.Time + CIObservedAt time.Time + ReviewObservedAt time.Time +} + +// PRCISummary describes the latest CI status and failing checks for a PR. +type PRCISummary struct { + State domain.CIState + FailingChecks []PRFailingCheck +} + +// PRFailingCheck is one failed or cancelled CI check for a PR. +type PRFailingCheck struct { + Name string + Status domain.PRCheckStatus + Conclusion string + URL string +} + +// PRReviewSummary describes the latest review decision and unresolved comments. +type PRReviewSummary struct { + Decision domain.ReviewDecision + HasUnresolvedHumanComments bool + UnresolvedBy []PRUnresolvedReviewer +} + +// PRUnresolvedReviewer groups unresolved human comments by reviewer. +type PRUnresolvedReviewer struct { + ReviewerID string + Count int + Links []PRReviewCommentLink +} + +// PRReviewCommentLink points to one unresolved review comment. +type PRReviewCommentLink struct { + URL string + File string + Line int +} + +// PRMergeabilitySummary describes whether a PR can be merged and why. +type PRMergeabilitySummary struct { + State domain.Mergeability + Reasons []string + PRURL string + ConflictFiles []PRConflictFile +} + +// PRConflictFile is one file involved in a PR merge conflict. +type PRConflictFile struct { + Path string + URL string +} + +// ListPRSummaries returns all PRs owned by a session with concise SCM details +// assembled from persisted PR/check/review facts. +func (s *Service) ListPRSummaries(ctx context.Context, id domain.SessionID) ([]PRSummary, error) { + if _, ok, err := s.store.GetSession(ctx, id); err != nil { + return nil, fmt.Errorf("get %s: %w", id, err) + } else if !ok { + return nil, apierr.NotFound("SESSION_NOT_FOUND", "Unknown session") + } + prs, err := s.store.ListPRsBySession(ctx, id) + if err != nil { + return nil, err + } + out := make([]PRSummary, 0, len(prs)) + for _, pr := range prs { + checks, err := s.store.ListChecks(ctx, pr.URL) + if err != nil { + return nil, err + } + threads, err := s.store.ListPRReviewThreads(ctx, pr.URL) + if err != nil { + return nil, err + } + comments, err := s.store.ListPRComments(ctx, pr.URL) + if err != nil { + return nil, err + } + out = append(out, summarizePR(pr, checks, threads, comments)) + } + sortPRSummaries(out) + return out, nil +} + +func summarizePR(pr domain.PullRequest, checks []domain.PullRequestCheck, threads []domain.PullRequestReviewThread, comments []domain.PullRequestComment) PRSummary { + return PRSummary{ + URL: pr.URL, + HTMLURL: firstNonEmpty(pr.HTMLURL, pr.URL), + Number: pr.Number, + Title: pr.Title, + State: pullRequestState(pr), + Provider: firstNonEmpty(pr.Provider, "github"), + Repo: pr.Repo, + Author: pr.Author, + SourceBranch: pr.SourceBranch, + TargetBranch: pr.TargetBranch, + HeadSHA: pr.HeadSHA, + Additions: pr.Additions, + Deletions: pr.Deletions, + ChangedFiles: pr.ChangedFiles, + CI: summarizeCI(pr, checks), + Review: summarizeReview(pr, comments), + Mergeability: summarizeMergeability(pr, threads), + UpdatedAt: pr.UpdatedAt, + ObservedAt: pr.ObservedAt, + CIObservedAt: pr.CIObservedAt, + ReviewObservedAt: pr.ReviewObservedAt, + } +} + +func summarizeCI(pr domain.PullRequest, checks []domain.PullRequestCheck) PRCISummary { + state := ciOrUnknown(pr.CI) + out := PRCISummary{State: state} + if state != domain.CIFailing || pr.Merged || pr.Closed { + return out + } + for _, ch := range checks { + if ch.Status != domain.PRCheckFailed && ch.Status != domain.PRCheckCancelled { + continue + } + if pr.HeadSHA != "" && ch.CommitHash != "" && !strings.EqualFold(ch.CommitHash, pr.HeadSHA) { + continue + } + out.FailingChecks = append(out.FailingChecks, PRFailingCheck{ + Name: ch.Name, + Status: ch.Status, + Conclusion: ch.Conclusion, + URL: ch.URL, + }) + } + return out +} + +func summarizeReview(pr domain.PullRequest, comments []domain.PullRequestComment) PRReviewSummary { + out := PRReviewSummary{Decision: reviewOrNone(pr.Review)} + if pr.Merged || pr.Closed { + return out + } + byReviewer := map[string]int{} + order := []string{} + links := map[string][]PRReviewCommentLink{} + for _, c := range comments { + if c.Resolved || c.IsBot { + continue + } + reviewer := strings.TrimSpace(c.Author) + if reviewer == "" { + reviewer = "unknown" + } + if _, ok := byReviewer[reviewer]; !ok { + order = append(order, reviewer) + } + byReviewer[reviewer]++ + links[reviewer] = append(links[reviewer], PRReviewCommentLink{ + URL: c.URL, + File: c.File, + Line: c.Line, + }) + } + sort.Strings(order) + for _, reviewer := range order { + out.UnresolvedBy = append(out.UnresolvedBy, PRUnresolvedReviewer{ + ReviewerID: reviewer, + Count: byReviewer[reviewer], + Links: links[reviewer], + }) + } + out.HasUnresolvedHumanComments = len(out.UnresolvedBy) > 0 + return out +} + +func summarizeMergeability(pr domain.PullRequest, _ []domain.PullRequestReviewThread) PRMergeabilitySummary { + return PRMergeabilitySummary{ + State: mergeabilityOrUnknown(pr.Mergeability), + Reasons: mergeabilityReasons(pr), + PRURL: firstNonEmpty(pr.HTMLURL, pr.URL), + } +} + +func mergeabilityReasons(pr domain.PullRequest) []string { + if pr.Merged || pr.Closed { + return nil + } + if pr.Mergeability != domain.MergeConflicting && pr.Mergeability != domain.MergeBlocked && pr.Mergeability != domain.MergeUnstable { + return nil + } + reasons := map[string]bool{} + add := func(reason string) { + if reason != "" { + reasons[reason] = true + } + } + if pr.Mergeability == domain.MergeConflicting || containsAny(pr.ProviderMergeable, "conflict", "dirty") || containsAny(pr.ProviderMergeStateStatus, "conflict", "dirty") { + add("conflicts") + } + if containsAny(pr.ProviderMergeStateStatus, "behind") { + add("behind_base") + } + if pr.Draft { + add("draft") + } + if pr.CI == domain.CIFailing { + add("ci_failing") + } + if pr.Review == domain.ReviewChangesRequest { + add("changes_requested") + } + if pr.Review == domain.ReviewRequired { + add("review_required") + } + if pr.Mergeability == domain.MergeBlocked && len(reasons) == 0 { + add("blocked_by_provider") + } + if pr.Mergeability == domain.MergeUnstable && len(reasons) == 0 { + add("blocked_by_provider") + } + out := make([]string, 0, len(reasons)) + for reason := range reasons { + out = append(out, reason) + } + sort.Strings(out) + return out +} + +func containsAny(s string, needles ...string) bool { + s = strings.ToLower(s) + for _, needle := range needles { + if strings.Contains(s, needle) { + return true + } + } + return false +} + +func sortPRSummaries(prs []PRSummary) { + sort.SliceStable(prs, func(i, j int) bool { + ia, ja := prSummaryActive(prs[i]), prSummaryActive(prs[j]) + if ia != ja { + return ia + } + return prs[i].UpdatedAt.After(prs[j].UpdatedAt) + }) +} + +func prSummaryActive(pr PRSummary) bool { + return pr.State != domain.PRStateMerged && pr.State != domain.PRStateClosed +} + +func pullRequestState(pr domain.PullRequest) domain.PRState { + switch { + case pr.Merged: + return domain.PRStateMerged + case pr.Closed: + return domain.PRStateClosed + case pr.Draft: + return domain.PRStateDraft + default: + return domain.PRStateOpen + } +} + +func ciOrUnknown(state domain.CIState) domain.CIState { + if state == "" { + return domain.CIUnknown + } + return state +} + +func reviewOrNone(decision domain.ReviewDecision) domain.ReviewDecision { + if decision == "" { + return domain.ReviewNone + } + return decision +} + +func mergeabilityOrUnknown(state domain.Mergeability) domain.Mergeability { + if state == "" { + return domain.MergeUnknown + } + return state +} diff --git a/backend/internal/service/session/service.go b/backend/internal/service/session/service.go index b2b48940..8e27ae64 100644 --- a/backend/internal/service/session/service.go +++ b/backend/internal/service/session/service.go @@ -22,6 +22,8 @@ type Store interface { GetDisplayPRFactsForSession(ctx context.Context, id domain.SessionID) (domain.PRFacts, bool, error) ListPRFactsForSession(ctx context.Context, id domain.SessionID) ([]domain.PRFacts, error) ListPRsBySession(ctx context.Context, sessionID domain.SessionID) ([]domain.PullRequest, error) + ListChecks(ctx context.Context, prURL string) ([]domain.PullRequestCheck, error) + ListPRReviewThreads(ctx context.Context, prURL string) ([]domain.PullRequestReviewThread, error) ListPRComments(ctx context.Context, prURL string) ([]domain.PullRequestComment, error) GetProject(ctx context.Context, id string) (domain.ProjectRecord, bool, error) } diff --git a/backend/internal/service/session/service_test.go b/backend/internal/service/session/service_test.go index 9432148b..ac414519 100644 --- a/backend/internal/service/session/service_test.go +++ b/backend/internal/service/session/service_test.go @@ -24,11 +24,21 @@ type fakeStore struct { sessions map[domain.SessionID]domain.SessionRecord pr map[domain.SessionID]domain.PRFacts projects map[string]domain.ProjectRecord + checks map[string][]domain.PullRequestCheck + threads map[string][]domain.PullRequestReviewThread + comments map[string][]domain.PullRequestComment num int } func newFakeStore() *fakeStore { - return &fakeStore{sessions: map[domain.SessionID]domain.SessionRecord{}, pr: map[domain.SessionID]domain.PRFacts{}, projects: map[string]domain.ProjectRecord{}} + return &fakeStore{ + sessions: map[domain.SessionID]domain.SessionRecord{}, + pr: map[domain.SessionID]domain.PRFacts{}, + projects: map[string]domain.ProjectRecord{}, + checks: map[string][]domain.PullRequestCheck{}, + threads: map[string][]domain.PullRequestReviewThread{}, + comments: map[string][]domain.PullRequestComment{}, + } } func (f *fakeStore) CreateSession(_ context.Context, rec domain.SessionRecord) (domain.SessionRecord, error) { @@ -93,8 +103,16 @@ func (f *fakeStore) ListPRFactsForSession(_ context.Context, id domain.SessionID return []domain.PRFacts{pr}, nil } -func (f *fakeStore) ListPRComments(context.Context, string) ([]domain.PullRequestComment, error) { - return nil, nil +func (f *fakeStore) ListChecks(_ context.Context, prURL string) ([]domain.PullRequestCheck, error) { + return append([]domain.PullRequestCheck(nil), f.checks[prURL]...), nil +} + +func (f *fakeStore) ListPRReviewThreads(_ context.Context, prURL string) ([]domain.PullRequestReviewThread, error) { + return append([]domain.PullRequestReviewThread(nil), f.threads[prURL]...), nil +} + +func (f *fakeStore) ListPRComments(_ context.Context, prURL string) ([]domain.PullRequestComment, error) { + return append([]domain.PullRequestComment(nil), f.comments[prURL]...), nil } func (f *fakeStore) GetProject(_ context.Context, id string) (domain.ProjectRecord, bool, error) { @@ -558,6 +576,192 @@ func TestListPRsOrdersActiveBeforeClosedThenUpdatedDesc(t *testing.T) { } } +func TestListPRSummariesOmitsRawLogsAndReviewBodies(t *testing.T) { + st := newFakeStore() + now := time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC) + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindWorker} + prURL := "https://github.com/acme/repo/pull/7" + stList := &multiPRFakeStore{fakeStore: st, prs: []domain.PullRequest{{ + URL: prURL, + HTMLURL: prURL, + SessionID: "mer-1", + Number: 7, + CI: domain.CIFailing, + Review: domain.ReviewChangesRequest, + Mergeability: domain.MergeConflicting, + Provider: "github", + Repo: "acme/repo", + Title: "Fix dashboard", + Author: "ada", + SourceBranch: "fix/dashboard", + TargetBranch: "main", + HeadSHA: "abc123", + ProviderMergeStateStatus: "dirty", + UpdatedAt: now, + ObservedAt: now.Add(-time.Minute), + CIObservedAt: now.Add(-time.Minute), + ReviewObservedAt: now.Add(-time.Minute), + }}} + stList.checks[prURL] = []domain.PullRequestCheck{ + {Name: "unit", Status: domain.PRCheckFailed, Conclusion: "failure", URL: "https://github.com/acme/repo/actions/runs/1", LogTail: "panic: secret"}, + {Name: "lint", Status: domain.PRCheckPassed, Conclusion: "success", URL: "https://github.com/acme/repo/actions/runs/2"}, + } + stList.comments[prURL] = []domain.PullRequestComment{ + {Author: "reviewer-a", File: "main.go", Line: 12, Body: "raw body must stay private", URL: "https://github.com/acme/repo/pull/7#discussion_r1"}, + {Author: "ci-bot", File: "main.go", Line: 13, Body: "bot body", URL: "https://github.com/acme/repo/pull/7#discussion_r2", IsBot: true}, + {Author: "reviewer-a", File: "test.go", Line: 22, Body: "another raw body", URL: "https://github.com/acme/repo/pull/7#discussion_r3"}, + } + + got, err := (&Service{store: stList}).ListPRSummaries(context.Background(), "mer-1") + if err != nil { + t.Fatal(err) + } + if len(got) != 1 { + t.Fatalf("summaries = %+v", got) + } + pr := got[0] + if pr.Title != "Fix dashboard" || pr.State != domain.PRStateOpen || pr.Provider != "github" || pr.Repo != "acme/repo" || pr.HeadSHA != "abc123" { + t.Fatalf("metadata = %+v", pr) + } + if len(pr.CI.FailingChecks) != 1 || pr.CI.FailingChecks[0].Name != "unit" || pr.CI.FailingChecks[0].URL == "" { + t.Fatalf("failing checks = %+v", pr.CI.FailingChecks) + } + if pr.Review.Decision != domain.ReviewChangesRequest || !pr.Review.HasUnresolvedHumanComments || len(pr.Review.UnresolvedBy) != 1 { + t.Fatalf("review = %+v", pr.Review) + } + if reviewer := pr.Review.UnresolvedBy[0]; reviewer.ReviewerID != "reviewer-a" || reviewer.Count != 2 || len(reviewer.Links) != 2 { + t.Fatalf("reviewer = %+v", reviewer) + } + if pr.Mergeability.State != domain.MergeConflicting || len(pr.Mergeability.ConflictFiles) != 0 || !containsString(pr.Mergeability.Reasons, "conflicts") { + t.Fatalf("mergeability = %+v", pr.Mergeability) + } +} + +func TestListPRSummariesSuppressesFailingChecksUnlessCIFailing(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindWorker} + prURL := "https://github.com/acme/repo/pull/8" + stList := &multiPRFakeStore{fakeStore: st, prs: []domain.PullRequest{{ + URL: prURL, + SessionID: "mer-1", + Number: 8, + CI: domain.CIPassing, + HeadSHA: "new-sha", + UpdatedAt: time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC), + }}} + stList.checks[prURL] = []domain.PullRequestCheck{ + {Name: "copy-check", CommitHash: "old-sha", Status: domain.PRCheckFailed, Conclusion: "failure", URL: "https://github.com/acme/repo/actions/runs/1"}, + } + + got, err := (&Service{store: stList}).ListPRSummaries(context.Background(), "mer-1") + if err != nil { + t.Fatal(err) + } + if got[0].CI.State != domain.CIPassing || len(got[0].CI.FailingChecks) != 0 { + t.Fatalf("ci summary = %+v", got[0].CI) + } +} + +func TestListPRSummariesFiltersFailedChecksToCurrentHead(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindWorker} + prURL := "https://github.com/acme/repo/pull/9" + stList := &multiPRFakeStore{fakeStore: st, prs: []domain.PullRequest{{ + URL: prURL, + SessionID: "mer-1", + Number: 9, + CI: domain.CIFailing, + HeadSHA: "new-sha", + UpdatedAt: time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC), + }}} + stList.checks[prURL] = []domain.PullRequestCheck{ + {Name: "old-copy-check", CommitHash: "old-sha", Status: domain.PRCheckFailed, Conclusion: "failure"}, + {Name: "current-lint", CommitHash: "new-sha", Status: domain.PRCheckFailed, Conclusion: "failure"}, + } + + got, err := (&Service{store: stList}).ListPRSummaries(context.Background(), "mer-1") + if err != nil { + t.Fatal(err) + } + checks := got[0].CI.FailingChecks + if len(checks) != 1 || checks[0].Name != "current-lint" { + t.Fatalf("failing checks = %+v", checks) + } +} + +func TestListPRSummariesSuppressesActiveDetailsForClosedOrMergedPRs(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindWorker} + prURL := "https://github.com/acme/repo/pull/10" + stList := &multiPRFakeStore{fakeStore: st, prs: []domain.PullRequest{{ + URL: prURL, + SessionID: "mer-1", + Number: 10, + Merged: true, + CI: domain.CIFailing, + Review: domain.ReviewChangesRequest, + Mergeability: domain.MergeConflicting, + ProviderMergeStateStatus: "dirty", + UpdatedAt: time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC), + }}} + stList.checks[prURL] = []domain.PullRequestCheck{{Name: "unit", Status: domain.PRCheckFailed}} + stList.comments[prURL] = []domain.PullRequestComment{{Author: "reviewer-a", File: "main.go", Line: 12, URL: "https://github.com/acme/repo/pull/10#discussion_r1"}} + + got, err := (&Service{store: stList}).ListPRSummaries(context.Background(), "mer-1") + if err != nil { + t.Fatal(err) + } + pr := got[0] + if pr.State != domain.PRStateMerged { + t.Fatalf("state = %q", pr.State) + } + if len(pr.CI.FailingChecks) != 0 || len(pr.Review.UnresolvedBy) != 0 || len(pr.Mergeability.Reasons) != 0 { + t.Fatalf("active details should be suppressed for merged PR: ci=%+v review=%+v merge=%+v", pr.CI, pr.Review, pr.Mergeability) + } +} + +func TestListPRSummariesOnlyEmitsMergeReasonsForBlockedStates(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Kind: domain.KindWorker} + now := time.Date(2026, 6, 4, 12, 0, 0, 0, time.UTC) + stList := &multiPRFakeStore{fakeStore: st, prs: []domain.PullRequest{ + { + URL: "mergeable", + SessionID: "mer-1", + Number: 11, + CI: domain.CIFailing, + Review: domain.ReviewRequired, + Mergeability: domain.MergeMergeable, + ProviderMergeStateStatus: "behind", + UpdatedAt: now, + }, + { + URL: "blocked", + SessionID: "mer-1", + Number: 12, + Review: domain.ReviewRequired, + Mergeability: domain.MergeBlocked, + ProviderMergeStateStatus: "behind", + UpdatedAt: now.Add(time.Minute), + }, + }} + + got, err := (&Service{store: stList}).ListPRSummaries(context.Background(), "mer-1") + if err != nil { + t.Fatal(err) + } + byNumber := map[int]PRSummary{} + for _, pr := range got { + byNumber[pr.Number] = pr + } + if reasons := byNumber[11].Mergeability.Reasons; len(reasons) != 0 { + t.Fatalf("mergeable reasons = %+v", reasons) + } + if reasons := byNumber[12].Mergeability.Reasons; !containsString(reasons, "behind_base") || !containsString(reasons, "review_required") { + t.Fatalf("blocked reasons = %+v", reasons) + } +} + type multiPRFakeStore struct { *fakeStore prs []domain.PullRequest @@ -566,3 +770,12 @@ type multiPRFakeStore struct { func (f *multiPRFakeStore) ListPRsBySession(context.Context, domain.SessionID) ([]domain.PullRequest, error) { return f.prs, nil } + +func containsString(values []string, want string) bool { + for _, got := range values { + if got == want { + return true + } + } + return false +} diff --git a/docs/STATUS.md b/docs/STATUS.md index 13cbbd37..9d0ec703 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -63,6 +63,14 @@ surface (`npm run sqlc`, `npm run api`). - Shell: sidebar (projects + sessions, add/remove project), sessions board, session view + inspector, project settings, pull-requests page, spawn-orchestrator flow. +- Desktop status and SCM summary V1: session status comes from + `GET /api/v1/sessions`; visible/active PR context comes from + `GET /api/v1/sessions/{sessionId}/pr`; `GET /api/v1/events` is kept open as + an invalidation stream rather than a full PR payload stream. +- Concise PR summaries include PR identity, CI state with failing check names + and links, human reviewer IDs/counts/links for unresolved review comments, + and mergeability reasons. Raw CI logs and review comment bodies are + intentionally not part of the desktop V1 API/UI. - Terminal pane (xterm) over the mux WebSocket, with a live SSE events connection and port-rebind on daemon restart. @@ -73,8 +81,9 @@ surface (`npm run sqlc`, `npm run api`). nothing at runtime ([#112](https://github.com/aoagents/agent-orchestrator/issues/112)). - **Notifications**: design/in-flight only; no shipped backend notifier or UI center. -- **Live PR/tracker fact surfacing**: the observer writes facts, but exposing - the full `pr_*` / `tracker_*` CDC events to live consumers +- **Full raw PR/tracker fact surfacing**: the SCM observer writes facts and the + desktop consumes concise PR summaries, but exposing the full raw `pr_*` / + `tracker_*` CDC events to live consumers ([#110](https://github.com/aoagents/agent-orchestrator/issues/110)) and in `ao session get` ([#111](https://github.com/aoagents/agent-orchestrator/issues/111)) is still open. diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index bd8f3337..6da28399 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -457,7 +457,8 @@ export interface components { kind: string; projectId: string; prs: components["schemas"]["SessionPRFacts"][]; - status: string; + /** @enum {string} */ + status: "working" | "pr_open" | "draft" | "ci_failed" | "review_pending" | "changes_requested" | "approved" | "mergeable" | "merged" | "needs_input" | "idle" | "terminated" | "no_signal"; terminalHandleId?: string; /** Format: date-time */ updatedAt: string; @@ -493,7 +494,7 @@ export interface components { reviews: components["schemas"]["ReviewRun"][]; }; ListSessionPRsResponse: { - prs: components["schemas"]["SessionPRFacts"][]; + prs: components["schemas"]["SessionPRSummary"][]; sessionId: string; }; ListSessionsResponse: { @@ -627,11 +628,23 @@ export interface components { ok: boolean; sessionId: string; }; + SessionPRCISummary: { + failingChecks: components["schemas"]["SessionPRFailingCheck"][]; + /** @enum {string} */ + state: "unknown" | "pending" | "passing" | "failing"; + }; + SessionPRConflictFile: { + path: string; + url?: string; + }; SessionPRFacts: { - ci: string; - mergeability: string; + /** @enum {string} */ + ci: "unknown" | "pending" | "passing" | "failing"; + /** @enum {string} */ + mergeability: "unknown" | "mergeable" | "conflicting" | "blocked" | "unstable"; number: number; - review: string; + /** @enum {string} */ + review: "none" | "approved" | "changes_requested" | "review_required"; reviewComments: boolean; /** @enum {string} */ state: "draft" | "open" | "merged" | "closed"; @@ -639,6 +652,65 @@ export interface components { updatedAt: string; url: string; }; + SessionPRFailingCheck: { + conclusion: string; + name: string; + /** @enum {string} */ + status: "failed" | "cancelled"; + url?: string; + }; + SessionPRMergeabilitySummary: { + conflictFiles?: components["schemas"]["SessionPRConflictFile"][]; + prUrl: string; + reasons: string[]; + /** @enum {string} */ + state: "unknown" | "mergeable" | "conflicting" | "blocked" | "unstable"; + }; + SessionPRReviewCommentLink: { + file?: string; + line?: number; + url?: string; + }; + SessionPRReviewSummary: { + /** @enum {string} */ + decision: "none" | "approved" | "changes_requested" | "review_required"; + hasUnresolvedHumanComments: boolean; + unresolvedBy: components["schemas"]["SessionPRUnresolvedReviewer"][]; + }; + SessionPRSummary: { + additions: number; + author: string; + changedFiles: number; + ci: components["schemas"]["SessionPRCISummary"]; + /** Format: date-time */ + ciObservedAt?: string; + deletions: number; + headSha: string; + htmlUrl?: string; + mergeability: components["schemas"]["SessionPRMergeabilitySummary"]; + number: number; + /** Format: date-time */ + observedAt?: string; + /** @enum {string} */ + provider: "github"; + repo: string; + review: components["schemas"]["SessionPRReviewSummary"]; + /** Format: date-time */ + reviewObservedAt?: string; + sourceBranch: string; + /** @enum {string} */ + state: "draft" | "open" | "merged" | "closed"; + targetBranch: string; + title: string; + /** Format: date-time */ + updatedAt: string; + url: string; + }; + SessionPRUnresolvedReviewer: { + count: number; + links: components["schemas"]["SessionPRReviewCommentLink"][]; + reviewerId: string; + }; SessionResponse: { session: components["schemas"]["ControllersSessionView"]; }; diff --git a/frontend/src/renderer/__tests__/integration/pr-hydration.test.tsx b/frontend/src/renderer/__tests__/integration/pr-hydration.test.tsx index f0d3f57f..d151c40b 100644 --- a/frontend/src/renderer/__tests__/integration/pr-hydration.test.tsx +++ b/frontend/src/renderer/__tests__/integration/pr-hydration.test.tsx @@ -23,8 +23,8 @@ vi.mock("@tanstack/react-router", async (importOriginal) => { import { SessionsBoard } from "../../components/SessionsBoard"; import { PullRequestsPage } from "../../components/PullRequestsPage"; -// One ordinary project with one worker session that has an open PR (#278). -function respondWithProjectAndPR() { +// One ordinary project with one worker session that has multiple PRs. +function respondWithProjectAndPRs() { getMock.mockImplementation(async (url: string) => { if (url === "/api/v1/projects") { return { data: { projects: [{ id: "proj-1", name: "my-app", path: "/repo/my-app" }] }, error: undefined }; @@ -52,6 +52,16 @@ function respondWithProjectAndPR() { reviewComments: false, updatedAt: "2026-06-10T16:15:04Z", }, + { + number: 279, + state: "draft", + url: "https://github.com/aoagents/ReverbCode/pull/279", + ci: "pending", + review: "pending", + mergeability: "unknown", + reviewComments: false, + updatedAt: "2026-06-10T16:20:04Z", + }, ], }, ], @@ -71,22 +81,24 @@ function renderWithProviders(node: ReactNode) { beforeEach(() => { getMock.mockReset(); navigateMock.mockReset(); - respondWithProjectAndPR(); + respondWithProjectAndPRs(); }); describe("PR hydration for a normal project (#251)", () => { - it("renders the PR on the Board card instead of 'no PR yet'", async () => { + it("renders every session PR on the Board card instead of 'no PR yet'", async () => { renderWithProviders(); expect(await screen.findByText("PR #278 · open")).toBeInTheDocument(); + expect(screen.getByText("PR #279 · draft")).toBeInTheDocument(); expect(screen.queryByText("no PR yet")).not.toBeInTheDocument(); }); - it("lists the session on the PR page instead of being empty", async () => { + it("lists every session PR on the PR page instead of being empty", async () => { renderWithProviders(); expect(await screen.findByText("#278")).toBeInTheDocument(); + expect(screen.getByText("#279")).toBeInTheDocument(); expect(screen.queryByText("No open pull requests.")).not.toBeInTheDocument(); - expect(screen.getByText("fix the bug")).toBeInTheDocument(); + expect(screen.getAllByText("fix the bug")).toHaveLength(2); }); }); diff --git a/frontend/src/renderer/components/PRSummaryDisplay.tsx b/frontend/src/renderer/components/PRSummaryDisplay.tsx new file mode 100644 index 00000000..3eb54f15 --- /dev/null +++ b/frontend/src/renderer/components/PRSummaryDisplay.tsx @@ -0,0 +1,179 @@ +import { ArrowUpDown, ArrowUpRight } from "lucide-react"; +import { Fragment, type ReactNode } from "react"; +import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; +import { prAttentionItems, prStatusRows, type PRAttentionLink, type PRDisplayTone } from "../lib/pr-display"; +import { cn } from "../lib/utils"; + +const toneClass: Record = { + neutral: "text-muted-foreground", + passive: "text-passive", + success: "text-success", + warning: "text-warning", + error: "text-error", +}; + +export function PRStatusStrip({ className, pr }: { className?: string; pr: SessionPRSummary }) { + return ( +
+ {prStatusRows(pr).map((row) => ( + + {row.label}{" "} + {row.value} + {row.detail ? · {row.detail} : null} + + ))} +
+ ); +} + +export function PRSummaryMeta({ + className, + leading, + pr, +}: { + className?: string; + leading?: string; + pr: SessionPRSummary; +}) { + const branchRange = prBranchRange(pr); + const hasDiff = hasDiffMetadata(pr); + const primary = [leading, branchRange, pr.author].filter(Boolean); + if (primary.length === 0 && !hasDiff) { + return null; + } + return ( +
+ {primary.length > 0 ?
{primary.join(" · ")}
: null} + {hasDiff ? : null} +
+ ); +} + +function PRDiffMeta({ pr }: { pr: SessionPRSummary }) { + const parts: ReactNode[] = []; + if (pr.changedFiles > 0) { + parts.push( + + , + ); + } + if (pr.additions > 0) { + parts.push( + + +{pr.additions} + , + ); + } + if (pr.deletions > 0) { + parts.push( + + -{pr.deletions} + , + ); + } + return ( +
+ {parts.map((part, index) => ( + + {index > 0 ? · : null} + {part} + + ))} +
+ ); +} + +export function PRAttentionPanel({ + className, + interactiveLinks = true, + maxItems = 3, + pr, +}: { + className?: string; + interactiveLinks?: boolean; + maxItems?: number; + pr: SessionPRSummary; +}) { + const items = prAttentionItems(pr); + if (items.length === 0) { + return null; + } + const visible = items.slice(0, maxItems); + const extra = items.length - visible.length; + return ( +
+
+ Needs attention +
+
+ {visible.map((item) => ( +
+
{item.title}
+ {item.summary ? ( +
{item.summary}
+ ) : null} + {item.links.length > 0 ? ( +
+ {item.links.map((link, index) => ( + + ))} + {item.overflowLabel ? {item.overflowLabel} : null} +
+ ) : null} +
+ ))} + {extra > 0 ?
+{extra} more
: null} +
+
+ ); +} + +function AttentionLink({ interactive, link }: { interactive: boolean; link: PRAttentionLink }) { + if (interactive && link.href) { + return ( + event.stopPropagation()} + rel="noopener noreferrer" + target="_blank" + title={link.title} + > + {link.label} + + ); + } + return ( + + {link.label} + + ); +} + +function prBranchRange(pr: SessionPRSummary): string | undefined { + if (pr.sourceBranch && pr.targetBranch) { + return `${pr.sourceBranch} -> ${pr.targetBranch}`; + } + if (pr.sourceBranch) { + return pr.sourceBranch; + } + if (pr.targetBranch) { + return `-> ${pr.targetBranch}`; + } + return undefined; +} + +function hasDiffMetadata(pr: SessionPRSummary): boolean { + return pr.changedFiles > 0 || pr.additions > 0 || pr.deletions > 0; +} + +function pluralize(noun: string, count: number): string { + return count === 1 ? noun : `${noun}s`; +} diff --git a/frontend/src/renderer/components/PullRequestsPage.tsx b/frontend/src/renderer/components/PullRequestsPage.tsx index bded8cf0..8bc76ab4 100644 --- a/frontend/src/renderer/components/PullRequestsPage.tsx +++ b/frontend/src/renderer/components/PullRequestsPage.tsx @@ -1,15 +1,24 @@ import { useNavigate } from "@tanstack/react-router"; -import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { useMutation, useQueries, useQueryClient } from "@tanstack/react-query"; import { useState } from "react"; import { apiClient, apiErrorMessage } from "../lib/api-client"; import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; -import { type PRState, type PullRequestFacts, type WorkspaceSession } from "../types/workspace"; +import { + sessionScmSummaryQueryKey, + sessionScmSummaryQueryOptions, + type SessionPRSummary, +} from "../hooks/useSessionScmSummary"; +import { comparePRDisplaySummaries, prDiffSummary, sessionPRDisplaySummaries } from "../lib/pr-display"; +import type { WorkspaceSession } from "../types/workspace"; import { DashboardSubhead } from "./DashboardSubhead"; import { Badge } from "./ui/badge"; import { Button } from "./ui/button"; +import { PRAttentionPanel, PRStatusStrip } from "./PRSummaryDisplay"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "./ui/table"; import { cn } from "../lib/utils"; +type PRState = SessionPRSummary["state"]; + const stateTone: Record = { open: "border-success/40 bg-success/10 text-success", draft: "border-border bg-raised text-muted-foreground", @@ -17,11 +26,8 @@ const stateTone: Record = { closed: "border-error/40 bg-error/10 text-error", }; -// Order open PRs (actionable) above merged/closed. -const stateRank: Record = { open: 0, draft: 1, merged: 2, closed: 3 }; - type PRRow = { - pr: PullRequestFacts; + pr: SessionPRSummary; session: WorkspaceSession; }; @@ -34,9 +40,14 @@ export function PullRequestsPage() { const navigate = useNavigate(); const workspaceQuery = useWorkspaceQuery(); const sessions = (workspaceQuery.data ?? []).flatMap((w) => w.sessions); + const prQueries = useQueries({ + queries: sessions.map((session) => sessionScmSummaryQueryOptions(session.id)), + }); const rows: PRRow[] = sessions - .flatMap((s) => s.prs.map((pr) => ({ pr, session: s }))) - .sort((a, b) => stateRank[a.pr.state] - stateRank[b.pr.state] || a.pr.number - b.pr.number); + .flatMap((session, index) => + sessionPRDisplaySummaries(session, prQueries[index]?.data).map((pr) => ({ pr, session })), + ) + .sort((a, b) => comparePRDisplaySummaries(a.pr, b.pr)); return (
@@ -83,7 +94,10 @@ export function PullRequestsPage() { function PRRowView({ row, onOpen }: { row: PRRow; onOpen: () => void }) { const queryClient = useQueryClient(); const [note, setNote] = useState<{ ok: boolean; text: string } | null>(null); - const refresh = () => void queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); + const refresh = () => { + void queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); + void queryClient.invalidateQueries({ queryKey: sessionScmSummaryQueryKey() }); + }; const merge = useMutation({ mutationFn: async () => { @@ -120,10 +134,19 @@ function PRRowView({ row, onOpen }: { row: PRRow; onOpen: () => void }) { #{row.pr.number} -
{row.session.title}
+
{row.pr.title || row.session.title}
- {[row.session.workspaceName, row.session.branch].filter(Boolean).join(" · ")} + {[ + row.session.workspaceName, + row.pr.sourceBranch || row.session.branch, + row.pr.targetBranch ? `-> ${row.pr.targetBranch}` : "", + prDiffSummary(row.pr), + ] + .filter(Boolean) + .join(" · ")}
+ +
diff --git a/frontend/src/renderer/components/SessionInspector.test.tsx b/frontend/src/renderer/components/SessionInspector.test.tsx index bb517969..41365e2a 100644 --- a/frontend/src/renderer/components/SessionInspector.test.tsx +++ b/frontend/src/renderer/components/SessionInspector.test.tsx @@ -108,7 +108,7 @@ describe("SessionInspector PR section", () => { within(screen.getByText(title).closest("section.inspector-section") as HTMLElement); it("renders one card per PR, ordered actionable-first, when a session owns a stack", () => { - render(); + renderWithQuery(); expect(screen.getByText("Pull requests (3)")).toBeInTheDocument(); const cards = prSection("Pull requests (3)") @@ -119,22 +119,22 @@ describe("SessionInspector PR section", () => { }); it("uses the singular heading and shows enriched facts for a single PR", () => { - render(); + renderWithQuery(); expect(screen.getByText("Pull request")).toBeInTheDocument(); expect(screen.queryByText(/Pull requests \(/)).not.toBeInTheDocument(); expect(prSection("Pull request").getByText("PR #7")).toBeInTheDocument(); // CI/Merge/Review facts surface per card. - expect(prSection("Pull request").getAllByText("passing").length).toBeGreaterThan(0); + expect(prSection("Pull request").getAllByText("Passing").length).toBeGreaterThan(0); }); it("shows the empty state when there are no PRs", () => { - render(); + renderWithQuery(); expect(screen.getByText("No pull request opened yet.")).toBeInTheDocument(); }); it("links each PR to its url", () => { - render(); + renderWithQuery(); const links = screen.getAllByRole("link", { name: /Open/ }); expect(links.map((a) => a.getAttribute("href"))).toEqual([ "https://example.com/pr/41", @@ -145,7 +145,7 @@ describe("SessionInspector PR section", () => { describe("SessionInspector tabs", () => { it("exposes Summary, Reviews, and Browser as the three inspector tabs", () => { - render(); + renderWithQuery(); const tabs = screen.getAllByRole("tab").map((el) => el.textContent?.trim()); expect(tabs).toEqual(["Summary", "Reviews", "Browser"]); }); diff --git a/frontend/src/renderer/components/SessionInspector.tsx b/frontend/src/renderer/components/SessionInspector.tsx index edd99b9e..efcb2b9d 100644 --- a/frontend/src/renderer/components/SessionInspector.tsx +++ b/frontend/src/renderer/components/SessionInspector.tsx @@ -1,14 +1,26 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useState, type ReactNode } from "react"; -import { AlertCircle, CheckCircle2, CircleMinus, GitPullRequest, Play, Shield, Terminal } from "lucide-react"; +import { + AlertCircle, + ArrowUpRight, + CheckCircle2, + CircleMinus, + GitPullRequest, + Play, + Shield, + Terminal, +} from "lucide-react"; import type { components } from "../../api/schema"; 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 { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; +import { prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display"; +import type { SessionStatus, WorkspaceSession } from "../types/workspace"; import { sortedPRs, workerDisplayStatus } from "../types/workspace"; import { Badge } from "./ui/badge"; import { cn } from "../lib/utils"; +import { PRAttentionPanel, PRSummaryMeta } from "./PRSummaryDisplay"; type ProjectConfig = components["schemas"]["ProjectConfig"]; type ReviewRun = components["schemas"]["ReviewRun"]; @@ -54,7 +66,7 @@ const VIEWS: { id: InspectorView; label: string; icon: ReactNode }[] = [ }, ]; -const prStateTone: Record = { +const prStateTone: Record = { open: "border-success/40 bg-success/10 text-success", draft: "border-border bg-raised text-muted-foreground", merged: "border-accent/40 bg-accent-weak text-accent", @@ -110,9 +122,19 @@ export function SessionInspector({ ); } -function Section({ title, action, children }: { title: string; action?: ReactNode; children: ReactNode }) { +function Section({ + action, + children, + className, + title, +}: { + action?: ReactNode; + children: ReactNode; + className?: string; + title: string; +}) { return ( -
+
{title} {action ?? null} @@ -123,18 +145,20 @@ function Section({ title, action, children }: { title: string; action?: ReactNod } function SummaryView({ session }: { session: WorkspaceSession }) { - const prs = sortedPRs(session); + const query = useSessionScmSummary(session.id); + const prSummaries = sessionPRDisplaySummaries(session, query.data); + const prSectionTitle = prSummaries.length > 1 ? `Pull requests (${prSummaries.length})` : "Pull request"; const branchLabel = session.branch || `session/${session.id}`; return (
-
1 ? `Pull requests (${prs.length})` : "Pull request"}> - {prs.length === 0 ? ( +
+ {prSummaries.length === 0 ? (

No pull request opened yet.

) : ( -
- {prs.map((pr) => ( - +
+ {prSummaries.map((pr) => ( + ))}
)} @@ -144,7 +168,7 @@ function SummaryView({ session }: { session: WorkspaceSession }) {
-
+
@@ -156,33 +180,61 @@ function SummaryView({ session }: { session: WorkspaceSession }) { ); } -// One PR per card; a session's PRs stack vertically. Mirrors the minimal -// single-PR rail the parallel-agent tools converged on (emdash, conductor), -// repeated per PR rather than collapsed into one aggregate widget. -function PRCard({ pr }: { pr: PullRequestFacts }) { +function PRSummaryCard({ pr }: { pr: SessionPRSummary }) { return ( -
+
-
- - - -
+ {pr.title ?
{pr.title}
: null} + + + +
+ ); +} + +function PRStatusStack({ className, pr }: { className?: string; pr: SessionPRSummary }) { + return ( +
+ {prStatusRows(pr).map((row) => ( +
+ {row.label}{" "} + {row.value} +
+ ))}
); } +function inspectorStatusToneClass(tone: PRDisplayTone): string { + switch (tone) { + case "success": + return "text-success"; + case "warning": + return "text-warning"; + case "error": + return "text-error"; + case "neutral": + return "text-muted-foreground"; + case "passive": + return "text-passive"; + } +} + type TimelineTone = "now" | "good" | "warn" | "neutral"; function ActivityTimeline({ session }: { session: WorkspaceSession }) { @@ -266,6 +318,7 @@ const STATUS_PILL: Record< no_signal: { label: "No signal", tone: "var(--fg-muted)", breathe: false }, mergeable: { label: "Ready", tone: "var(--green)", breathe: false }, done: { label: "Done", tone: "var(--fg-muted)", breathe: false }, + unknown: { label: "Unknown", tone: "var(--fg-muted)", breathe: false }, idle: { label: "Idle", tone: "var(--fg-muted)", breathe: false }, }; diff --git a/frontend/src/renderer/components/SessionsBoard.tsx b/frontend/src/renderer/components/SessionsBoard.tsx index 6014b02b..65b8e493 100644 --- a/frontend/src/renderer/components/SessionsBoard.tsx +++ b/frontend/src/renderer/components/SessionsBoard.tsx @@ -2,13 +2,16 @@ import { useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "@tanstack/react-router"; import { Plus } from "lucide-react"; -import { type AttentionZone, type WorkspaceSession, attentionZone, openPRs, workerSessions } from "../types/workspace"; +import { type AttentionZone, type WorkspaceSession, attentionZone, workerSessions } from "../types/workspace"; +import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; import { useWorkspaceQuery, workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { DashboardSubhead } from "./DashboardSubhead"; import { OrchestratorIcon } from "./icons"; import { NewTaskDialog } from "./NewTaskDialog"; import { spawnOrchestrator } from "../lib/spawn-orchestrator"; +import { prDiffSummary, sessionPRDisplaySummaries } from "../lib/pr-display"; import { cn } from "../lib/utils"; +import { PRAttentionPanel, PRStatusStrip } from "./PRSummaryDisplay"; type SessionsBoardProps = { /** When set, the board shows only this project's sessions. */ @@ -255,23 +258,11 @@ function ZoneColumn({ ); } -// One-line PR summary for the card footer. A session can own several PRs, so -// collapse to a count once past one; detail lives in the inspector stack. -function prSummary(session: WorkspaceSession): string { - const total = session.prs.length; - if (total === 0) return "no PR yet"; - if (total === 1) { - const pr = session.prs[0]; - return `PR #${pr.number} · ${pr.state}`; - } - const open = openPRs(session).length; - return open > 0 ? `${total} PRs · ${open} open` : `${total} PRs`; -} - function SessionCard({ session, onOpen }: { session: WorkspaceSession; onOpen: () => void }) { const badge = sessionBadge(session); const branch = session.branch || ""; const showBranch = branch !== "" && !sameLabel(branch, session.title) && !sameLabel(branch, session.id); + const prSummaries = sessionPRDisplaySummaries(session, useSessionScmSummary(session.id).data); return ( ); } +function BoardPRSummary({ className, pr }: { className?: string; pr: SessionPRSummary }) { + const diffSummary = prDiffSummary(pr); + return ( +
+ + PR #{pr.number} · {pr.state} + + {diffSummary ? {diffSummary} : null} + + +
+ ); +} + function sameLabel(a: string, b: string): boolean { const normalize = (value: string) => value diff --git a/frontend/src/renderer/components/ShellTopbar.tsx b/frontend/src/renderer/components/ShellTopbar.tsx index 8ab3eec9..3a895a84 100644 --- a/frontend/src/renderer/components/ShellTopbar.tsx +++ b/frontend/src/renderer/components/ShellTopbar.tsx @@ -33,6 +33,7 @@ const STATUS_PILL: Record diff --git a/frontend/src/renderer/hooks/useSessionScmSummary.ts b/frontend/src/renderer/hooks/useSessionScmSummary.ts new file mode 100644 index 00000000..5b26eb8f --- /dev/null +++ b/frontend/src/renderer/hooks/useSessionScmSummary.ts @@ -0,0 +1,36 @@ +import { useQuery } from "@tanstack/react-query"; +import type { components } from "../../api/schema"; +import { apiClient } from "../lib/api-client"; + +export type SessionPRSummary = components["schemas"]["SessionPRSummary"]; + +export const sessionScmSummaryQueryKey = (sessionId?: string) => + sessionId ? (["session-scm-summary", sessionId] as const) : (["session-scm-summary"] as const); + +const usePreviewData = import.meta.env.VITE_NO_ELECTRON === "1"; + +export async function fetchSessionScmSummary(sessionId: string): Promise { + const { data, error } = await apiClient.GET("/api/v1/sessions/{sessionId}/pr", { + params: { path: { sessionId } }, + }); + if (error) throw error; + return data?.prs ?? []; +} + +export function sessionScmSummaryQueryOptions(sessionId: string) { + return { + queryKey: sessionScmSummaryQueryKey(sessionId), + enabled: Boolean(sessionId) && !usePreviewData, + queryFn: () => fetchSessionScmSummary(sessionId), + retry: 1, + }; +} + +export function useSessionScmSummary(sessionId?: string) { + return useQuery({ + queryKey: sessionScmSummaryQueryKey(sessionId), + enabled: Boolean(sessionId) && !usePreviewData, + queryFn: () => fetchSessionScmSummary(sessionId!), + retry: 1, + }); +} diff --git a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx index 692d51d2..a81309c1 100644 --- a/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx +++ b/frontend/src/renderer/hooks/useWorkspaceQuery.test.tsx @@ -70,7 +70,7 @@ describe("useWorkspaceQuery", () => { }, { // Unknown harness/status and no displayName/issueId: falls back - // to codex / working / the session id. + // to codex / unknown / the session id. id: "sess-2", projectId: "proj-1", harness: "mystery-agent", @@ -104,8 +104,8 @@ describe("useWorkspaceQuery", () => { id: "sess-2", title: "sess-2", provider: "codex", + status: "unknown", branch: "session/sess-2", - status: "working", }); }); @@ -167,7 +167,7 @@ describe("useWorkspaceQuery", () => { expect(sessions[1].prs).toEqual([]); }); - it("marks terminated sessions regardless of their reported status", async () => { + it("preserves backend merged status for terminated merged sessions", async () => { respondWith({ projects: { data: { projects: [{ id: "proj-1", name: "my-app", path: "/p" }] }, error: undefined }, sessions: { @@ -176,7 +176,32 @@ describe("useWorkspaceQuery", () => { { id: "sess-1", projectId: "proj-1", - status: "working", + status: "merged", + isTerminated: true, + updatedAt: "2026-06-10T16:15:04Z", + }, + ], + }, + error: undefined, + }, + }); + + const { result } = renderHook(() => useWorkspaceQuery(), { wrapper }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + + expect(result.current.data?.[0].sessions[0].status).toBe("merged"); + }); + + it("falls back to terminated for terminated sessions without a known backend status", async () => { + respondWith({ + projects: { data: { projects: [{ id: "proj-1", name: "my-app", path: "/p" }] }, error: undefined }, + sessions: { + data: { + sessions: [ + { + id: "sess-1", + projectId: "proj-1", + status: "bogus", isTerminated: true, updatedAt: "2026-06-10T16:15:04Z", }, diff --git a/frontend/src/renderer/lib/event-transport.test.ts b/frontend/src/renderer/lib/event-transport.test.ts index 90a59900..71e4f601 100644 --- a/frontend/src/renderer/lib/event-transport.test.ts +++ b/frontend/src/renderer/lib/event-transport.test.ts @@ -119,7 +119,7 @@ describe("createEventTransport", () => { expect(getEventsConnectionState()).toBe("disconnected"); }); - it("debounces a workspace invalidation after a status change", () => { + it("debounces workspace and SCM summary invalidation after a status change", () => { vi.useFakeTimers(); try { const queryClient = fakeQueryClient(); @@ -129,7 +129,8 @@ describe("createEventTransport", () => { onStatusHandler(); expect(queryClient.invalidateQueries).not.toHaveBeenCalled(); vi.advanceTimersByTime(200); - expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(1); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: ["workspaces"] }); + expect(queryClient.invalidateQueries).toHaveBeenCalledWith({ queryKey: ["session-scm-summary"] }); } finally { vi.useRealTimers(); } diff --git a/frontend/src/renderer/lib/event-transport.ts b/frontend/src/renderer/lib/event-transport.ts index feb9f138..a1682ce8 100644 --- a/frontend/src/renderer/lib/event-transport.ts +++ b/frontend/src/renderer/lib/event-transport.ts @@ -3,6 +3,7 @@ import { aoBridge } from "./bridge"; import { getApiBaseUrl, hasTrustedApiBaseUrl, subscribeApiBaseUrl } from "./api-client"; import { setEventsConnectionState } from "./events-connection"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; +import { sessionScmSummaryQueryKey } from "../hooks/useSessionScmSummary"; export type EventTransport = { connect: () => () => void; @@ -50,6 +51,7 @@ export function createEventTransport(queryClient: QueryClient): EventTransport { if (debounce) clearTimeout(debounce); debounce = setTimeout(() => { void queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); + void queryClient.invalidateQueries({ queryKey: sessionScmSummaryQueryKey() }); }, INVALIDATE_DEBOUNCE_MS); }; diff --git a/frontend/src/renderer/lib/pr-display.test.ts b/frontend/src/renderer/lib/pr-display.test.ts new file mode 100644 index 00000000..fd50f591 --- /dev/null +++ b/frontend/src/renderer/lib/pr-display.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from "vitest"; +import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; +import { prAttentionItems, prDiffSummary, prStatusRows } from "./pr-display"; + +const summary = (overrides: Partial = {}): SessionPRSummary => ({ + url: "https://github.com/acme/repo/pull/7", + htmlUrl: "https://github.com/acme/repo/pull/7", + number: 7, + title: "Fix dashboard", + state: "open", + provider: "github", + repo: "acme/repo", + author: "ada", + sourceBranch: "fix/dashboard", + targetBranch: "main", + headSha: "abc123", + additions: 10, + deletions: 3, + changedFiles: 2, + ci: { state: "passing", failingChecks: [] }, + review: { decision: "approved", hasUnresolvedHumanComments: false, unresolvedBy: [] }, + mergeability: { state: "mergeable", reasons: [], prUrl: "https://github.com/acme/repo/pull/7" }, + updatedAt: "2026-06-15T00:00:00Z", + observedAt: "2026-06-15T00:00:00Z", + ciObservedAt: "2026-06-15T00:00:00Z", + reviewObservedAt: "2026-06-15T00:00:00Z", + ...overrides, +}); + +describe("prStatusRows", () => { + it("formats the three PR states without exposing raw unknown", () => { + const rows = prStatusRows( + summary({ + ci: { state: "unknown", failingChecks: [] }, + review: { decision: "none", hasUnresolvedHumanComments: false, unresolvedBy: [] }, + mergeability: { state: "unknown", reasons: [], prUrl: "https://github.com/acme/repo/pull/7" }, + }), + ); + + expect(rows.map((row) => `${row.label}:${row.value}`)).toEqual(["CI:Checking", "Review:None", "Merge:Checking"]); + }); + + it("includes minimal diff detail on the merge row", () => { + const rows = prStatusRows(summary({ changedFiles: 4, additions: 25, deletions: 2 })); + expect(rows.find((row) => row.key === "merge")?.detail).toBe("4 files"); + }); +}); + +describe("prDiffSummary", () => { + it("formats file and line delta metadata", () => { + expect(prDiffSummary(summary({ changedFiles: 6, additions: 42, deletions: 8 }))).toBe("6 files · +42 -8"); + }); + + it("omits the diff label when no diff metadata is available", () => { + expect(prDiffSummary(summary({ changedFiles: 0, additions: 0, deletions: 0 }))).toBeUndefined(); + }); +}); + +describe("prAttentionItems", () => { + it("returns no attention for clean open PRs", () => { + expect(prAttentionItems(summary())).toEqual([]); + }); + + it("details active CI, review, and merge blockers", () => { + const items = prAttentionItems( + summary({ + ci: { + state: "failing", + failingChecks: [ + { name: "copy-check", status: "failed", conclusion: "failure", url: "https://checks.example/copy" }, + ], + }, + review: { + decision: "changes_requested", + hasUnresolvedHumanComments: true, + unresolvedBy: [ + { + reviewerId: "alice", + count: 6, + links: [{ url: "https://github.com/acme/repo/pull/7#discussion_r1", file: "main.go", line: 12 }], + }, + ], + }, + mergeability: { + state: "blocked", + reasons: ["behind_base"], + prUrl: "https://github.com/acme/repo/pull/7", + }, + }), + ); + + expect(items.map((item) => item.kind)).toEqual(["merge_blocked", "ci_failing", "review_changes_requested"]); + expect(items.find((item) => item.kind === "ci_failing")?.links[0]).toMatchObject({ + label: "copy-check", + href: "https://checks.example/copy", + }); + expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({ + label: "alice +5", + href: "https://github.com/acme/repo/pull/7#discussion_r1", + }); + }); + + it("suppresses attention once the PR is closed or merged", () => { + expect( + prAttentionItems( + summary({ + state: "merged", + ci: { state: "failing", failingChecks: [{ name: "unit", status: "failed", conclusion: "failure" }] }, + review: { decision: "changes_requested", hasUnresolvedHumanComments: true, unresolvedBy: [] }, + mergeability: { state: "conflicting", reasons: ["conflicts"], prUrl: "https://github.com/acme/repo/pull/7" }, + }), + ), + ).toEqual([]); + }); +}); diff --git a/frontend/src/renderer/lib/pr-display.ts b/frontend/src/renderer/lib/pr-display.ts new file mode 100644 index 00000000..b533c0d4 --- /dev/null +++ b/frontend/src/renderer/lib/pr-display.ts @@ -0,0 +1,393 @@ +import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; +import { sortedPRs, type PRState, type PullRequestFacts, type WorkspaceSession } from "../types/workspace"; + +const prStateRank: Record = { open: 0, draft: 1, merged: 2, closed: 3 }; +const ciStates = new Set(["unknown", "pending", "passing", "failing"]); +const reviewDecisions = new Set([ + "none", + "approved", + "changes_requested", + "review_required", +]); +const mergeabilityStates = new Set([ + "unknown", + "mergeable", + "conflicting", + "blocked", + "unstable", +]); + +export type PRDisplayTone = "neutral" | "passive" | "success" | "warning" | "error"; + +export type PRStatusRow = { + key: "ci" | "review" | "merge"; + label: string; + value: string; + detail?: string; + tone: PRDisplayTone; +}; + +export type PRAttentionLink = { + label: string; + href?: string; + title?: string; +}; + +export type PRAttentionItem = { + kind: "draft" | "ci_failing" | "review_changes_requested" | "review_pending" | "merge_conflict" | "merge_blocked"; + title: string; + summary?: string; + links: PRAttentionLink[]; + overflowLabel?: string; + tone: PRDisplayTone; +}; + +export function comparePRDisplaySummaries(a: SessionPRSummary, b: SessionPRSummary): number { + return prStateRank[a.state] - prStateRank[b.state] || a.number - b.number; +} + +export function sessionPRDisplaySummaries( + session: WorkspaceSession, + summaries: SessionPRSummary[] = [], +): SessionPRSummary[] { + const summariesByNumber = new Map(summaries.map((summary) => [summary.number, summary])); + const seen = new Set(); + const fromFacts = sortedPRs(session).map((pr) => { + seen.add(pr.number); + return summariesByNumber.get(pr.number) ?? sessionPRFactToSummary(session, pr); + }); + const summaryOnly = summaries.filter((summary) => !seen.has(summary.number)); + return [...fromFacts, ...summaryOnly].sort(comparePRDisplaySummaries); +} + +function sessionPRFactToSummary(session: WorkspaceSession, pr: PullRequestFacts): SessionPRSummary { + return { + url: pr.url, + htmlUrl: pr.url, + number: pr.number, + title: session.title, + state: pr.state, + provider: "github", + repo: session.workspaceName, + author: "", + sourceBranch: session.branch, + targetBranch: "", + headSha: "", + additions: 0, + deletions: 0, + changedFiles: 0, + ci: { + state: toCIState(pr.ci), + failingChecks: [], + }, + review: { + decision: toReviewDecision(pr.review), + hasUnresolvedHumanComments: pr.reviewComments, + unresolvedBy: [], + }, + mergeability: { + state: toMergeabilityState(pr.mergeability), + reasons: [], + prUrl: pr.url, + conflictFiles: [], + }, + updatedAt: pr.updatedAt, + observedAt: pr.updatedAt, + ciObservedAt: pr.updatedAt, + reviewObservedAt: pr.updatedAt, + }; +} + +export function prStatusRows(pr: SessionPRSummary): PRStatusRow[] { + return [ + { + key: "ci", + label: "CI", + value: ciLabel(pr.ci.state), + tone: ciTone(pr.ci.state), + }, + { + key: "review", + label: "Review", + value: reviewLabel(pr.review.decision), + tone: reviewTone(pr.review.decision, pr.review.hasUnresolvedHumanComments), + }, + { + key: "merge", + label: "Merge", + value: mergeabilityLabel(pr.mergeability.state), + detail: formatDiffSummary(pr), + tone: mergeabilityTone(pr.mergeability.state), + }, + ]; +} + +export function prDiffSummary(pr: SessionPRSummary): string | undefined { + const parts: string[] = []; + if (pr.changedFiles > 0) { + parts.push(`${pr.changedFiles} ${pluralize("file", pr.changedFiles)}`); + } + const lineDelta = formatLineDelta(pr.additions, pr.deletions); + if (lineDelta) { + parts.push(lineDelta); + } + return parts.length > 0 ? parts.join(" · ") : undefined; +} + +export function prAttentionItems(pr: SessionPRSummary): PRAttentionItem[] { + if (pr.state === "merged" || pr.state === "closed") { + return []; + } + if (pr.state === "draft") { + return [ + { + kind: "draft", + title: "Draft PR", + summary: "Not ready for review", + links: [], + tone: "passive", + }, + ]; + } + + const items: PRAttentionItem[] = []; + if (pr.mergeability.state === "conflicting") { + items.push( + mergeAttention(pr, "merge_conflict", "Resolve merge conflict", "Conflicts with the base branch", "error"), + ); + } else if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") { + items.push(mergeAttention(pr, "merge_blocked", "Merge blocked", "Provider reports merge is blocked", "warning")); + } + if (pr.ci.state === "failing") { + const links = pr.ci.failingChecks.slice(0, 3).map((check) => ({ + label: check.name, + href: check.url || undefined, + title: check.conclusion || check.status, + })); + items.push({ + kind: "ci_failing", + title: "Fix failing CI", + summary: links.length === 0 ? "No failing check link observed" : undefined, + links, + overflowLabel: overflowLabel(pr.ci.failingChecks.length, 3, "check"), + tone: "error", + }); + } + if (pr.review.decision === "changes_requested" || pr.review.hasUnresolvedHumanComments) { + const reviewers = pr.review.unresolvedBy.slice(0, 3); + const links = reviewers.map((reviewer) => ({ + label: reviewerLabel(reviewer), + href: reviewer.links.find((link) => link.url)?.url || undefined, + title: `${reviewer.count} unresolved ${pluralize("comment", reviewer.count)}`, + })); + items.push({ + kind: "review_changes_requested", + title: "Address requested changes", + summary: links.length === 0 ? "Requested changes still active" : undefined, + links, + overflowLabel: overflowLabel(pr.review.unresolvedBy.length, 3, "reviewer"), + tone: "warning", + }); + } else if (pr.review.decision === "review_required") { + items.push({ + kind: "review_pending", + title: "Review pending", + summary: "Required review not submitted", + links: [], + tone: "neutral", + }); + } + return items; +} + +function toCIState(value: string): SessionPRSummary["ci"]["state"] { + return ciStates.has(value as SessionPRSummary["ci"]["state"]) + ? (value as SessionPRSummary["ci"]["state"]) + : "unknown"; +} + +function toReviewDecision(value: string): SessionPRSummary["review"]["decision"] { + return reviewDecisions.has(value as SessionPRSummary["review"]["decision"]) + ? (value as SessionPRSummary["review"]["decision"]) + : "none"; +} + +function toMergeabilityState(value: string): SessionPRSummary["mergeability"]["state"] { + return mergeabilityStates.has(value as SessionPRSummary["mergeability"]["state"]) + ? (value as SessionPRSummary["mergeability"]["state"]) + : "unknown"; +} + +function ciLabel(state: SessionPRSummary["ci"]["state"]): string { + switch (state) { + case "passing": + return "Passing"; + case "failing": + return "Failing"; + case "pending": + return "Pending"; + case "unknown": + return "Checking"; + } +} + +function ciTone(state: SessionPRSummary["ci"]["state"]): PRDisplayTone { + switch (state) { + case "passing": + return "success"; + case "failing": + return "error"; + case "pending": + return "neutral"; + case "unknown": + return "passive"; + } +} + +function reviewLabel(decision: SessionPRSummary["review"]["decision"]): string { + switch (decision) { + case "approved": + return "Approved"; + case "changes_requested": + return "Changes requested"; + case "review_required": + return "Pending"; + case "none": + return "None"; + } +} + +function reviewTone( + decision: SessionPRSummary["review"]["decision"], + hasUnresolvedHumanComments: boolean, +): PRDisplayTone { + switch (decision) { + case "approved": + return "success"; + case "changes_requested": + return "warning"; + case "review_required": + return "neutral"; + case "none": + return hasUnresolvedHumanComments ? "warning" : "passive"; + } +} + +function mergeabilityLabel(state: SessionPRSummary["mergeability"]["state"]): string { + switch (state) { + case "mergeable": + return "Mergeable"; + case "conflicting": + return "Conflict"; + case "blocked": + return "Blocked"; + case "unstable": + return "Unstable"; + case "unknown": + return "Checking"; + } +} + +function mergeabilityTone(state: SessionPRSummary["mergeability"]["state"]): PRDisplayTone { + switch (state) { + case "mergeable": + return "success"; + case "conflicting": + return "error"; + case "blocked": + case "unstable": + return "warning"; + case "unknown": + return "passive"; + } +} + +function formatDiffSummary(pr: SessionPRSummary): string | undefined { + if (pr.changedFiles > 0) { + return `${pr.changedFiles} ${pluralize("file", pr.changedFiles)}`; + } + const changedLines = pr.additions + pr.deletions; + if (changedLines > 0) { + return `${changedLines} ${pluralize("line", changedLines)}`; + } + return undefined; +} + +function formatLineDelta(additions: number, deletions: number): string | undefined { + const parts: string[] = []; + if (additions > 0) { + parts.push(`+${additions}`); + } + if (deletions > 0) { + parts.push(`-${deletions}`); + } + return parts.length > 0 ? parts.join(" ") : undefined; +} + +function mergeAttention( + pr: SessionPRSummary, + kind: Extract, + title: string, + fallback: string, + tone: PRDisplayTone, +): PRAttentionItem { + const fileLinks = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({ + label: file.path, + href: file.url || pr.mergeability.prUrl || undefined, + })); + const reasonLinks = + fileLinks.length > 0 + ? [] + : pr.mergeability.reasons.slice(0, 3).map((reason) => ({ + label: mergeReasonLabel(reason), + href: pr.mergeability.prUrl || undefined, + })); + const links = fileLinks.length > 0 ? fileLinks : reasonLinks; + return { + kind, + title, + summary: links.length === 0 ? fallback : undefined, + links, + overflowLabel: + fileLinks.length > 0 + ? overflowLabel(pr.mergeability.conflictFiles?.length ?? 0, 3, "file") + : overflowLabel(pr.mergeability.reasons.length, 3, "reason"), + tone, + }; +} + +function reviewerLabel(reviewer: SessionPRSummary["review"]["unresolvedBy"][number]): string { + if (reviewer.count <= 1) { + return reviewer.reviewerId; + } + return `${reviewer.reviewerId} +${reviewer.count - 1}`; +} + +function mergeReasonLabel(reason: string): string { + switch (reason) { + case "behind_base": + return "branch behind base"; + case "ci_failing": + return "CI failing"; + case "changes_requested": + return "changes requested"; + case "review_required": + return "review required"; + case "blocked_by_provider": + return "provider blocked"; + default: + return reason.replaceAll("_", " "); + } +} + +function overflowLabel(total: number, shown: number, noun: string): string | undefined { + const extra = total - shown; + if (extra <= 0) { + return undefined; + } + return `+${extra} ${pluralize(noun, extra)}`; +} + +function pluralize(noun: string, count: number): string { + return count === 1 ? noun : `${noun}s`; +} diff --git a/frontend/src/renderer/routes/_shell.tsx b/frontend/src/renderer/routes/_shell.tsx index 7c26c176..87615604 100644 --- a/frontend/src/renderer/routes/_shell.tsx +++ b/frontend/src/renderer/routes/_shell.tsx @@ -1,4 +1,4 @@ -import { createFileRoute, Outlet, useNavigate, useRouterState } from "@tanstack/react-router"; +import { createFileRoute, Outlet, useNavigate } from "@tanstack/react-router"; import { useQueryClient } from "@tanstack/react-query"; import { type CSSProperties, useCallback, useEffect } from "react"; import { ShellTopbar } from "../components/ShellTopbar"; @@ -40,13 +40,11 @@ function errorMessage(error: unknown) { // instead of Zustand. The daemon-status effect runs here exactly once. function ShellLayout() { const navigate = useNavigate(); - const pathname = useRouterState({ select: (state) => state.location.pathname }); const queryClient = useQueryClient(); const workspaceQuery = useWorkspaceQuery(); const workspaces = workspaceQuery.data ?? []; const daemonStatus = useDaemonStatus(queryClient); const { theme, setTheme, isSidebarOpen, toggleSidebar } = useUiStore(); - const isBoardRoute = pathname === "/" || /^\/projects\/[^/]+$/.test(pathname); const updateWorkspaces = useCallback( (updater: (workspaces: WorkspaceSummary[]) => WorkspaceSummary[]) => { @@ -155,7 +153,7 @@ function ShellLayout() { in the layout, not the screens, so the crumb and actions never shift when the outlet content swaps. */}
- {!isBoardRoute && } + {/* Controlled by the ui-store so TitlebarNav / Topbar toggles (which call the store directly) stay in sync. --sidebar-width chains to the drag-resizable --ao-sidebar-w set on :root by useResizable. */} @@ -167,7 +165,7 @@ function ShellLayout() { > { expect(toSessionStatus("no_signal")).toBe("no_signal"); }); - it("overrides to terminated when the session is terminated", () => { - expect(toSessionStatus("working", true)).toBe("terminated"); + it("keeps a backend merged status even when the session is terminated", () => { + expect(toSessionStatus("merged", true)).toBe("merged"); }); - it("falls back to working for an unknown status", () => { - expect(toSessionStatus("bogus")).toBe("working"); + it("uses terminated only as a fallback when a terminated session has no known status", () => { + expect(toSessionStatus(undefined, true)).toBe("terminated"); }); - it("falls back to working when status is undefined", () => { - expect(toSessionStatus(undefined)).toBe("working"); + it("falls back to unknown for an unknown live status", () => { + expect(toSessionStatus("bogus")).toBe("unknown"); + expect(toSessionStatus(undefined)).toBe("unknown"); }); }); @@ -79,6 +80,7 @@ describe("workerDisplayStatus", () => { ["mergeable", "mergeable"], ["merged", "done"], ["terminated", "done"], + ["unknown", "unknown"], ["working", "working"], ["idle", "working"], ] as const)("maps %s to %s", (status, expected) => { @@ -130,9 +132,12 @@ describe("findProjectOrchestrator", () => { }); describe("sessionNeedsAttention", () => { - it.each(["needs_input", "changes_requested", "review_pending", "ci_failed"] as const)("is true for %s", (status) => { - expect(sessionNeedsAttention(sessionWith({ status }))).toBe(true); - }); + it.each(["needs_input", "no_signal", "changes_requested", "review_pending", "ci_failed"] as const)( + "is true for %s", + (status) => { + expect(sessionNeedsAttention(sessionWith({ status }))).toBe(true); + }, + ); it("treats no_signal as needing attention", () => { expect(sessionNeedsAttention(sessionWith({ status: "no_signal" }))).toBe(true); @@ -151,6 +156,7 @@ describe("workerStatusPulses", () => { expect(workerStatusPulses("mergeable")).toBe(false); expect(workerStatusPulses("no_signal")).toBe(false); expect(workerStatusPulses("done")).toBe(false); + expect(workerStatusPulses("unknown")).toBe(false); }); }); @@ -205,12 +211,13 @@ describe("attentionZone", () => { ["mergeable", "merge"], ["approved", "merge"], ["needs_input", "action"], - ["ci_failed", "action"], ["no_signal", "action"], + ["ci_failed", "action"], ["changes_requested", "action"], ["review_pending", "pending"], ["pr_open", "pending"], ["draft", "pending"], + ["unknown", "pending"], ["working", "working"], ["idle", "working"], ["merged", "done"], diff --git a/frontend/src/renderer/types/workspace.ts b/frontend/src/renderer/types/workspace.ts index 47918246..f9203864 100644 --- a/frontend/src/renderer/types/workspace.ts +++ b/frontend/src/renderer/types/workspace.ts @@ -11,7 +11,8 @@ export type SessionStatus = | "needs_input" | "no_signal" | "idle" - | "terminated"; + | "terminated" + | "unknown"; const sessionStatuses = new Set([ "working", @@ -30,8 +31,8 @@ const sessionStatuses = new Set([ ]); export function toSessionStatus(status?: string, isTerminated = false): SessionStatus { - if (isTerminated) return "terminated"; - return status && sessionStatuses.has(status as SessionStatus) ? (status as SessionStatus) : "working"; + if (status && sessionStatuses.has(status as SessionStatus)) return status as SessionStatus; + return isTerminated ? "terminated" : "unknown"; } export type AgentProvider = @@ -121,7 +122,14 @@ export type WorkspaceSession = { }; /** Glanceable worker status. Maps 1:1 to the accent colors in DESIGN.md. */ -export type WorkerDisplayStatus = "working" | "needs_you" | "mergeable" | "ci_failed" | "no_signal" | "done"; +export type WorkerDisplayStatus = + | "working" + | "needs_you" + | "mergeable" + | "ci_failed" + | "no_signal" + | "done" + | "unknown"; export function workerDisplayStatus(session: WorkspaceSession): WorkerDisplayStatus { if (session.displayStatus) return session.displayStatus; @@ -140,6 +148,8 @@ export function workerDisplayStatus(session: WorkspaceSession): WorkerDisplaySta case "merged": case "terminated": return "done"; + case "unknown": + return "unknown"; default: return "working"; } @@ -212,6 +222,7 @@ export const workerStatusLabel: Record = { ci_failed: "ci failed", no_signal: "no signal", done: "done", + unknown: "unknown", }; /** Whether a status should breathe (alive/working). */ @@ -260,6 +271,7 @@ export function attentionZone(session: WorkspaceSession): AttentionZone { case "review_pending": case "pr_open": case "draft": + case "unknown": return "pending"; // Agents doing their thing — don't interrupt. case "working":