refactor: replace magic strings with constants for status enums (#64)
Replaces hardcoded string literals with typed constants for: - SESSION_STATUS (needs_input, stuck, errored, etc.) - PR_STATE (merged, closed, open) - CI_STATUS (passing, failing, pending, none) Benefits: - Type safety - IDE catches typos at compile time - Autocomplete - Better developer experience - Single source of truth - Easy to add/remove states - Easier refactoring - Change constant value in one place - Better git grep - Search for SESSION_STATUS.STUCK vs all "stuck" strings Files updated: - packages/core/src/types.ts - Added constants - packages/core/src/lifecycle-manager.ts - SessionStatus, PRState, CIStatus - packages/core/src/session-manager.ts - PRState - packages/web/src/lib/types.ts - SessionStatus, CIStatus - packages/web/src/components/SessionCard.tsx - CIStatus - packages/web/src/components/SessionDetail.tsx - CIStatus - packages/web/src/components/Dashboard.tsx - CIStatus - packages/plugins/scm-github/src/index.ts - CIStatus - packages/plugins/notifier-slack/src/index.ts - CIStatus Follows pattern established in PR #55 for ACTIVITY_STATE constants. Resolves INT-1368 Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
a6866458ec
commit
1cb52c122d
|
|
@ -11,23 +11,26 @@
|
|||
*/
|
||||
|
||||
import { randomUUID } from "node:crypto";
|
||||
import type {
|
||||
LifecycleManager,
|
||||
SessionManager,
|
||||
SessionId,
|
||||
SessionStatus,
|
||||
EventType,
|
||||
OrchestratorEvent,
|
||||
OrchestratorConfig,
|
||||
ReactionConfig,
|
||||
ReactionResult,
|
||||
PluginRegistry,
|
||||
Runtime,
|
||||
Agent,
|
||||
SCM,
|
||||
Notifier,
|
||||
Session,
|
||||
EventPriority,
|
||||
import {
|
||||
SESSION_STATUS,
|
||||
PR_STATE,
|
||||
CI_STATUS,
|
||||
type LifecycleManager,
|
||||
type SessionManager,
|
||||
type SessionId,
|
||||
type SessionStatus,
|
||||
type EventType,
|
||||
type OrchestratorEvent,
|
||||
type OrchestratorConfig,
|
||||
type ReactionConfig,
|
||||
type ReactionResult,
|
||||
type PluginRegistry,
|
||||
type Runtime,
|
||||
type Agent,
|
||||
type SCM,
|
||||
type Notifier,
|
||||
type Session,
|
||||
type EventPriority,
|
||||
} from "./types.js";
|
||||
import { updateMetadata } from "./metadata.js";
|
||||
|
||||
|
|
@ -214,7 +217,7 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
} catch {
|
||||
// On probe failure, preserve current stuck/needs_input state rather
|
||||
// than letting the fallback at the bottom coerce them to "working"
|
||||
if (session.status === "stuck" || session.status === "needs_input") {
|
||||
if (session.status === SESSION_STATUS.STUCK || session.status === SESSION_STATUS.NEEDS_INPUT) {
|
||||
return session.status;
|
||||
}
|
||||
}
|
||||
|
|
@ -224,12 +227,12 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
if (session.pr && scm) {
|
||||
try {
|
||||
const prState = await scm.getPRState(session.pr);
|
||||
if (prState === "merged") return "merged";
|
||||
if (prState === "closed") return "killed";
|
||||
if (prState === PR_STATE.MERGED) return "merged";
|
||||
if (prState === PR_STATE.CLOSED) return "killed";
|
||||
|
||||
// Check CI
|
||||
const ciStatus = await scm.getCISummary(session.pr);
|
||||
if (ciStatus === "failing") return "ci_failed";
|
||||
if (ciStatus === CI_STATUS.FAILING) return "ci_failed";
|
||||
|
||||
// Check reviews
|
||||
const reviewDecision = await scm.getReviewDecision(session.pr);
|
||||
|
|
@ -251,8 +254,8 @@ export function createLifecycleManager(deps: LifecycleManagerDeps): LifecycleMan
|
|||
// 4. Default: if agent is active, it's working
|
||||
if (
|
||||
session.status === "spawning" ||
|
||||
session.status === "stuck" ||
|
||||
session.status === "needs_input"
|
||||
session.status === SESSION_STATUS.STUCK ||
|
||||
session.status === SESSION_STATUS.NEEDS_INPUT
|
||||
) {
|
||||
return "working";
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import {
|
|||
type PluginRegistry,
|
||||
type RuntimeHandle,
|
||||
type Issue,
|
||||
PR_STATE,
|
||||
} from "./types.js";
|
||||
import {
|
||||
readMetadataRaw,
|
||||
|
|
@ -548,7 +549,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager {
|
|||
if (session.pr && plugins.scm) {
|
||||
try {
|
||||
const prState = await plugins.scm.getPRState(session.pr);
|
||||
if (prState === "merged" || prState === "closed") {
|
||||
if (prState === PR_STATE.MERGED || prState === PR_STATE.CLOSED) {
|
||||
shouldKill = true;
|
||||
}
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -58,6 +58,26 @@ export const ACTIVITY_STATE = {
|
|||
EXITED: "exited" as const,
|
||||
} satisfies Record<string, ActivityState>;
|
||||
|
||||
/** Session status constants */
|
||||
export const SESSION_STATUS = {
|
||||
SPAWNING: "spawning" as const,
|
||||
WORKING: "working" as const,
|
||||
PR_OPEN: "pr_open" as const,
|
||||
CI_FAILED: "ci_failed" as const,
|
||||
REVIEW_PENDING: "review_pending" as const,
|
||||
CHANGES_REQUESTED: "changes_requested" as const,
|
||||
APPROVED: "approved" as const,
|
||||
MERGEABLE: "mergeable" as const,
|
||||
MERGED: "merged" as const,
|
||||
CLEANUP: "cleanup" as const,
|
||||
NEEDS_INPUT: "needs_input" as const,
|
||||
STUCK: "stuck" as const,
|
||||
ERRORED: "errored" as const,
|
||||
KILLED: "killed" as const,
|
||||
DONE: "done" as const,
|
||||
TERMINATED: "terminated" as const,
|
||||
} satisfies Record<string, SessionStatus>;
|
||||
|
||||
/** A running agent session */
|
||||
export interface Session {
|
||||
/** Unique session ID, e.g. "my-app-3" */
|
||||
|
|
@ -452,6 +472,13 @@ export interface PRInfo {
|
|||
|
||||
export type PRState = "open" | "merged" | "closed";
|
||||
|
||||
/** PR state constants */
|
||||
export const PR_STATE = {
|
||||
OPEN: "open" as const,
|
||||
MERGED: "merged" as const,
|
||||
CLOSED: "closed" as const,
|
||||
} satisfies Record<string, PRState>;
|
||||
|
||||
export type MergeMethod = "merge" | "squash" | "rebase";
|
||||
|
||||
// --- CI Types ---
|
||||
|
|
@ -467,6 +494,14 @@ export interface CICheck {
|
|||
|
||||
export type CIStatus = "pending" | "passing" | "failing" | "none";
|
||||
|
||||
/** CI status constants */
|
||||
export const CI_STATUS = {
|
||||
PENDING: "pending" as const,
|
||||
PASSING: "passing" as const,
|
||||
FAILING: "failing" as const,
|
||||
NONE: "none" as const,
|
||||
} satisfies Record<string, CIStatus>;
|
||||
|
||||
// --- Review Types ---
|
||||
|
||||
export interface Review {
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
type NotifyAction,
|
||||
type NotifyContext,
|
||||
type EventPriority,
|
||||
CI_STATUS,
|
||||
} from "@composio/ao-core";
|
||||
|
||||
export const manifest = {
|
||||
|
|
@ -65,7 +66,7 @@ function buildBlocks(event: OrchestratorEvent, actions?: NotifyAction[]): unknow
|
|||
// Add CI status if available (type-guarded)
|
||||
const ciStatus = typeof event.data.ciStatus === "string" ? event.data.ciStatus : undefined;
|
||||
if (ciStatus) {
|
||||
const ciEmoji = ciStatus === "passing" ? ":white_check_mark:" : ":x:";
|
||||
const ciEmoji = ciStatus === CI_STATUS.PASSING ? ":white_check_mark:" : ":x:";
|
||||
blocks.push({
|
||||
type: "context",
|
||||
elements: [
|
||||
|
|
|
|||
|
|
@ -6,21 +6,22 @@
|
|||
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import type {
|
||||
PluginModule,
|
||||
SCM,
|
||||
Session,
|
||||
ProjectConfig,
|
||||
PRInfo,
|
||||
PRState,
|
||||
MergeMethod,
|
||||
CICheck,
|
||||
CIStatus,
|
||||
Review,
|
||||
ReviewDecision,
|
||||
ReviewComment,
|
||||
AutomatedComment,
|
||||
MergeReadiness,
|
||||
import {
|
||||
CI_STATUS,
|
||||
type PluginModule,
|
||||
type SCM,
|
||||
type Session,
|
||||
type ProjectConfig,
|
||||
type PRInfo,
|
||||
type PRState,
|
||||
type MergeMethod,
|
||||
type CICheck,
|
||||
type CIStatus,
|
||||
type Review,
|
||||
type ReviewDecision,
|
||||
type ReviewComment,
|
||||
type AutomatedComment,
|
||||
type MergeReadiness,
|
||||
} from "@composio/ao-core";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
|
@ -525,7 +526,7 @@ function createGitHubSCM(): SCM {
|
|||
|
||||
// CI
|
||||
const ciStatus = await this.getCISummary(pr);
|
||||
const ciPassing = ciStatus === "passing" || ciStatus === "none";
|
||||
const ciPassing = ciStatus === CI_STATUS.PASSING || ciStatus === CI_STATUS.NONE;
|
||||
if (!ciPassing) {
|
||||
blockers.push(`CI is ${ciStatus}`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
type AttentionLevel,
|
||||
getAttentionLevel,
|
||||
} from "@/lib/types";
|
||||
import { CI_STATUS } from "@composio/ao-core/types";
|
||||
import { AttentionZone } from "./AttentionZone";
|
||||
import { PRTableRow } from "./PRStatus";
|
||||
|
||||
|
|
@ -193,8 +194,8 @@ function mergeScore(
|
|||
): number {
|
||||
let score = 0;
|
||||
if (!pr.mergeability.noConflicts) score += 40;
|
||||
if (pr.ciStatus === "failing") score += 30;
|
||||
else if (pr.ciStatus === "pending") score += 5;
|
||||
if (pr.ciStatus === CI_STATUS.FAILING) score += 30;
|
||||
else if (pr.ciStatus === CI_STATUS.PENDING) score += 5;
|
||||
if (pr.reviewDecision === "changes_requested") score += 20;
|
||||
else if (pr.reviewDecision !== "approved") score += 10;
|
||||
score += pr.unresolvedThreads * 5;
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
import { useState, useEffect, useRef } from "react";
|
||||
import { type DashboardSession, type AttentionLevel, getAttentionLevel } from "@/lib/types";
|
||||
import { CI_STATUS } from "@composio/ao-core/types";
|
||||
import { cn } from "@/lib/cn";
|
||||
import { PRStatus } from "./PRStatus";
|
||||
import { CICheckList } from "./CIBadge";
|
||||
|
|
@ -311,7 +312,7 @@ function getAlerts(session: DashboardSession): Alert[] {
|
|||
const alerts: Alert[] = [];
|
||||
|
||||
// CI failing
|
||||
if (pr.ciStatus === "failing") {
|
||||
if (pr.ciStatus === CI_STATUS.FAILING) {
|
||||
const failedCheck = pr.ciChecks.find((c) => c.status === "failed");
|
||||
const failCount = pr.ciChecks.filter((c) => c.status === "failed").length;
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import {
|
|||
type DashboardSession,
|
||||
type DashboardPR,
|
||||
} from "@/lib/types";
|
||||
import { CI_STATUS } from "@composio/ao-core/types";
|
||||
import { CICheckList } from "./CIBadge";
|
||||
import { DirectTerminal } from "./DirectTerminal";
|
||||
|
||||
|
|
@ -417,7 +418,7 @@ function PRCard({ pr, sessionId }: { pr: DashboardPR; sessionId: string }) {
|
|||
function IssuesList({ pr }: { pr: DashboardPR }) {
|
||||
const issues: Array<{ icon: string; color: string; text: string }> = [];
|
||||
|
||||
if (pr.ciStatus === "failing") {
|
||||
if (pr.ciStatus === CI_STATUS.FAILING) {
|
||||
const failCount = pr.ciChecks.filter((c) => c.status === "failed").length;
|
||||
const text = failCount > 0
|
||||
? `CI failing \u2014 ${failCount} check${failCount !== 1 ? "s" : ""} failed`
|
||||
|
|
@ -427,7 +428,7 @@ function IssuesList({ pr }: { pr: DashboardPR }) {
|
|||
color: "var(--color-accent-red)",
|
||||
text,
|
||||
});
|
||||
} else if (pr.ciStatus === "pending") {
|
||||
} else if (pr.ciStatus === CI_STATUS.PENDING) {
|
||||
issues.push({
|
||||
icon: "\u25CF",
|
||||
color: "var(--color-accent-yellow)",
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ export type {
|
|||
|
||||
import {
|
||||
ACTIVITY_STATE,
|
||||
SESSION_STATUS,
|
||||
CI_STATUS,
|
||||
type CICheck as CoreCICheck,
|
||||
type MergeReadiness,
|
||||
type CIStatus,
|
||||
|
|
@ -171,9 +173,9 @@ export function getAttentionLevel(session: DashboardSession): AttentionLevel {
|
|||
return "respond";
|
||||
}
|
||||
if (
|
||||
session.status === "needs_input" ||
|
||||
session.status === "stuck" ||
|
||||
session.status === "errored"
|
||||
session.status === SESSION_STATUS.NEEDS_INPUT ||
|
||||
session.status === SESSION_STATUS.STUCK ||
|
||||
session.status === SESSION_STATUS.ERRORED
|
||||
) {
|
||||
return "respond";
|
||||
}
|
||||
|
|
@ -188,7 +190,7 @@ export function getAttentionLevel(session: DashboardSession): AttentionLevel {
|
|||
}
|
||||
if (session.pr) {
|
||||
const pr = session.pr;
|
||||
if (pr.ciStatus === "failing") return "review";
|
||||
if (pr.ciStatus === CI_STATUS.FAILING) return "review";
|
||||
if (pr.reviewDecision === "changes_requested") return "review";
|
||||
if (!pr.mergeability.noConflicts) return "review";
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue