From 1cb52c122d22353b5a6e65c596ead8ad7ea398a9 Mon Sep 17 00:00:00 2001 From: prateek Date: Mon, 16 Feb 2026 08:53:17 +0530 Subject: [PATCH] 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 --- packages/core/src/lifecycle-manager.ts | 49 ++++++++++--------- packages/core/src/session-manager.ts | 3 +- packages/core/src/types.ts | 35 +++++++++++++ packages/plugins/notifier-slack/src/index.ts | 3 +- packages/plugins/scm-github/src/index.ts | 33 +++++++------ packages/web/src/components/Dashboard.tsx | 5 +- packages/web/src/components/SessionCard.tsx | 3 +- packages/web/src/components/SessionDetail.tsx | 5 +- packages/web/src/lib/types.ts | 10 ++-- 9 files changed, 96 insertions(+), 50 deletions(-) diff --git a/packages/core/src/lifecycle-manager.ts b/packages/core/src/lifecycle-manager.ts index b2930dc98..22fe64c19 100644 --- a/packages/core/src/lifecycle-manager.ts +++ b/packages/core/src/lifecycle-manager.ts @@ -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"; } diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 02ab3d979..d8154516e 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -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 { diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 6349a1176..54ffae540 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -58,6 +58,26 @@ export const ACTIVITY_STATE = { EXITED: "exited" as const, } satisfies Record; +/** 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; + /** 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; + 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; + // --- Review Types --- export interface Review { diff --git a/packages/plugins/notifier-slack/src/index.ts b/packages/plugins/notifier-slack/src/index.ts index 6426c78c3..949ec505b 100644 --- a/packages/plugins/notifier-slack/src/index.ts +++ b/packages/plugins/notifier-slack/src/index.ts @@ -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: [ diff --git a/packages/plugins/scm-github/src/index.ts b/packages/plugins/scm-github/src/index.ts index 2b01c4ee8..97354e7bd 100644 --- a/packages/plugins/scm-github/src/index.ts +++ b/packages/plugins/scm-github/src/index.ts @@ -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}`); } diff --git a/packages/web/src/components/Dashboard.tsx b/packages/web/src/components/Dashboard.tsx index 01e618a64..b7432fdae 100644 --- a/packages/web/src/components/Dashboard.tsx +++ b/packages/web/src/components/Dashboard.tsx @@ -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; diff --git a/packages/web/src/components/SessionCard.tsx b/packages/web/src/components/SessionCard.tsx index 940de3ec4..ca1a94309 100644 --- a/packages/web/src/components/SessionCard.tsx +++ b/packages/web/src/components/SessionCard.tsx @@ -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; diff --git a/packages/web/src/components/SessionDetail.tsx b/packages/web/src/components/SessionDetail.tsx index 9c0204e52..0b12bc898 100644 --- a/packages/web/src/components/SessionDetail.tsx +++ b/packages/web/src/components/SessionDetail.tsx @@ -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)", diff --git a/packages/web/src/lib/types.ts b/packages/web/src/lib/types.ts index 696020e6c..ad274d9fb 100644 --- a/packages/web/src/lib/types.ts +++ b/packages/web/src/lib/types.ts @@ -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"; }