Skip to content

EDM-4999: Redefine which statuses allow start/stop actions #763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
celdrake wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −1
Open

EDM-4999: Redefine which statuses allow start/stop actions #763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion libs/ui-components/src/components/DetailsPage/Tables/ApplicationLifecycleActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
type ApplicationLifecycleAction,
hasAplicationStatusMismatch,
startableStatuses,
stoppableStatuses,

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove the duplicate stoppableStatuses declaration.

The component imports stoppableStatuses at Line 24. The supplied graph context also shows export const stoppableStatuses in this module at Lines 35-39. TypeScript rejects the duplicate import/local binding, so the build cannot compile.

Keep the utility export as the single source of truth. Remove the component-local declaration and migrate any external consumers to the utility export.

Proposed fix 
-export const stoppableStatuses = [
-  ApplicationStatusType.ApplicationStatusRunning,
-  ApplicationStatusType.ApplicationStatusError,
-  ApplicationStatusType.ApplicationStatusUnknown,
-];
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@libs/ui-components/src/components/DetailsPage/Tables/ApplicationLifecycleActions.tsx`
at line 24, Remove the component-local stoppableStatuses declaration in
ApplicationLifecycleActions and retain the imported utility export as the sole
source of truth. Update any consumers in this module to use the imported
stoppableStatuses binding, ensuring the duplicate declaration is not exported or
referenced.

transitionalStatuses,
} from '../../../utils/applicationLifecycle';
import {
Expand Down Expand Up @@ -107,6 +108,7 @@ const ApplicationLifecycleActions = ({
const isAppRunning = appStatus === ApplicationStatusType.ApplicationStatusRunning;
const desiredStateIsStopped = desiredState === ApplicationDesiredState.ApplicationDesiredStateStopped;
const canStart = startableStatuses.includes(appStatus);
const canStop = stoppableStatuses.includes(appStatus);
const isTransitioning = transitionalStatuses.includes(appStatus);
const hasStatusMismatch = hasAplicationStatusMismatch(appStatus, desiredState);
const isUserInitiatedTransition = pendingAction != null;
Expand Down Expand Up @@ -162,7 +164,7 @@ const ApplicationLifecycleActions = ({
{t('Start')}
</DropdownItem>
)}
{isAppRunning && (
{canStop && (
<DropdownItem component="button" onClick={() => handleAction('stop')}>
{t('Stop')}
</DropdownItem>
Expand Down
7 changes: 7 additions & 0 deletions libs/ui-components/src/utils/applicationLifecycle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,13 @@ export const startableStatuses = [
ApplicationStatusType.ApplicationStatusStopping,
ApplicationStatusType.ApplicationStatusStopped,
ApplicationStatusType.ApplicationStatusError,
ApplicationStatusType.ApplicationStatusUnknown,
];

export const stoppableStatuses = [
ApplicationStatusType.ApplicationStatusRunning,
ApplicationStatusType.ApplicationStatusError,
ApplicationStatusType.ApplicationStatusUnknown,
Comment on lines +32 to +38

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not use startableStatuses as the stop-completion predicate.

ApplicationStatusUnknown is now included in startableStatuses, but shouldClearPendingLifecycleAction uses that collection for pendingAction === 'stop' at Line 174. A stop issued while the application remains ApplicationStatusUnknown is therefore cleared immediately. This can re-enable the action and allow duplicate stop requests before a stopped state is observed.

Keep ApplicationStatusUnknown in stoppableStatuses, but use a dedicated stop-completion collection or explicitly exclude it from the stop branch. Add a regression test for a pending stop with current status ApplicationStatusUnknown.

Minimal fix 
     case 'stop':
-      return startableStatuses.includes(currentStatus);
+      return (
+        startableStatuses.includes(currentStatus) &&
+        currentStatus !== ApplicationStatusType.ApplicationStatusUnknown
+      );
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/ui-components/src/utils/applicationLifecycle.ts` around lines 32 - 38,
Update shouldClearPendingLifecycleAction so the pendingAction === 'stop' branch
uses stoppableStatuses, or another predicate that excludes
ApplicationStatusUnknown, rather than startableStatuses; keep
ApplicationStatusUnknown in stoppableStatuses and add a regression test covering
a pending stop while the current status is unknown.

];

export type ApplicationLifecycleAction = 'start' | 'stop' | 'restart';
Expand Down
Loading