Add queue management dashboard #15202
Conversation
- Add queue detail pages at /admin-panel/health-status/worker/:queueName - Display paginated job lists with state filtering (completed, failed, active, waiting, delayed, paused) - Add checkbox selection for bulk actions (delete, retry) - Add per-row dropdown menu with retry/delete options for individual jobs - Add expandable job details showing error messages, stack traces, and job data - Update job retention policy: 4 hours for completed, 7 days for failed (max 1000 each) - Add JobState enum for type-safe job state handling - Display dynamic retention configuration from backend
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR introduces a comprehensive queue management dashboard to the admin panel, enabling administrators to view, filter, and manage background jobs across BullMQ queues. The implementation spans both backend and frontend, adding a new AdminPanelQueueService with GraphQL endpoints for job listing, retry, and delete operations. The frontend receives a new queue detail page with paginated job tables supporting state-based filtering (completed, failed, active, waiting, delayed, paused), bulk operations via checkbox selection, and expandable rows displaying detailed job metadata including errors, stack traces, and execution data. The change also updates queue retention policies from immediate deletion to time-based retention (4 hours for completed jobs, 7 days for failed jobs, max 1000 each) and introduces a JobState enum for type safety. The architecture follows existing patterns: the service layer handles BullMQ interactions, resolvers expose GraphQL endpoints protected by admin guards, and the frontend uses hooks for data fetching with snackbar notifications for user feedback.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel-queue.service.ts | 2/5 | New service implementing queue management operations with critical issues: unsafe non-null assertion on job.id, incorrect pagination hasMore logic, missing paused state handling, and per-request Queue instantiation instead of connection pooling |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/QueueJobsTable.tsx | 2/5 | Core table component for job management with critical React violation: side effects executed directly in render body (lines 157-159) instead of useEffect, plus logic inconsistency in retry functionality |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/hooks/useDeleteJobs.ts | 2/5 | Delete jobs hook with critical lingui syntax errors using JavaScript template literal syntax ${variable} instead of lingui's {variable} format in translation macros |
| packages/twenty-front/src/pages/settings/admin-panel/SettingsAdminQueueDetail.tsx | 3/5 | Queue detail page with formatDuration function that incorrectly handles fractional values and hardcoded pluralization strings that bypass translation system |
| packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel.resolver.ts | 3/5 | Resolver adding queue management endpoints with unsafe type casting of queueName to MessageQueue enum without validation, and incorrect GraphQL Number type for pagination parameters |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/hooks/useRetryJobs.ts | 3/5 | Retry jobs hook with issue where onSuccess callback fires even when zero jobs are retried (error case), potentially causing unwanted side effects |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/RetryJobsConfirmationModal.tsx | 3/5 | Confirmation modal with hardcoded English pluralization strings that won't be properly translated by lingui |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/DeleteJobsConfirmationModal.tsx | 3/5 | Confirmation modal with hardcoded English pluralization strings bypassing translation system |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/graphql/queries/getQueueJobs.ts | 3.5/5 | GraphQL query using Float type for limit/offset pagination parameters instead of standard Int type |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/JobStateBadge.tsx | 4/5 | Badge component displaying job states with attempt counts, but uses danger styling for all attempt badges regardless of job state |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/JobDetailsExpandable.tsx | 4/5 | Expandable details component using array index as React key for logs rendering instead of stable identifiers |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/QueueJobRowDropdownMenu.tsx | 4/5 | Per-job dropdown menu missing dropdownHotkeyScope prop that other dropdown components include |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/components/WorkerQueueMetricsSection.tsx | 4/5 | Adds "View Jobs" navigation button to queue metrics with proper routing integration |
| packages/twenty-server/src/engine/core-modules/message-queue/drivers/bullmq.driver.ts | 4/5 | Centralizes retention policy constants for consistent job cleanup across regular and cron jobs |
| packages/twenty-shared/src/types/SettingsPath.ts | 4.5/5 | Adds new AdminPanelQueueDetail route path with dynamic queueName parameter |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/graphql/mutations/deleteJobs.ts | 4.5/5 | GraphQL mutation for bulk job deletion with properly typed required parameters |
| packages/twenty-front/src/modules/settings/admin-panel/health-status/graphql/mutations/retryJobs.ts | 4/5 | GraphQL mutation for bulk job retry with properly typed required parameters |
| packages/twenty-server/src/engine/core-modules/admin-panel/dtos/queue-job.dto.ts | 4/5 | GraphQL DTO for job data using GraphQLJSON for dynamic payloads and nullable fields for optional metadata |
| packages/twenty-front/src/modules/app/components/SettingsRoutes.tsx | 5/5 | Adds lazy-loaded route for queue detail page following existing patterns |
| packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel.module.ts | 5/5 | Registers AdminPanelQueueService in module providers for dependency injection |
| packages/twenty-server/src/engine/core-modules/admin-panel/dtos/queue-jobs-response.dto.ts | 5/5 | Pagination response DTO with job list, metadata, and retention configuration |
| packages/twenty-server/src/engine/core-modules/admin-panel/enums/job-state.enum.ts | 5/5 | Defines JobState enum matching BullMQ states with GraphQL registration |
| packages/twenty-server/src/engine/core-modules/admin-panel/dtos/queue-retention-config.dto.ts | 5/5 | Exposes queue retention configuration via GraphQL for frontend display |
| packages/twenty-server/src/engine/core-modules/message-queue/utils/queue-retention.constants.ts | 5/5 | Centralizes retention policy constants (4 hours for completed, 7 days for failed jobs) |
| packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel-health.service.ts | 5/5 | Updates Redis client call to use dedicated queue-specific connection method |
| packages/twenty-server/src/engine/core-modules/health/indicators/worker.health.ts | 4/5 | Updates Redis client method call from getClient() to getQueueClient() for queue operations |
| packages/twenty-front/src/generated/graphql.ts | 5/5 | Generated GraphQL types including JobState enum, queue job queries/mutations, and supporting types |
Confidence score: 2/5
- This PR contains critical bugs that will cause runtime failures and should not be merged without fixes
- Score reflects multiple high-severity issues: React rules violation causing potential infinite loops (QueueJobsTable line 157-159), lingui syntax errors breaking translations (useDeleteJobs), unsafe non-null assertions and incorrect pagination logic (AdminPanelQueueService), GraphQL schema type mismatches (Float vs Int for pagination), and hardcoded untranslatable strings in multiple confirmation modals
- Pay close attention to packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel-queue.service.ts, packages/twenty-front/src/modules/settings/admin-panel/health-status/components/QueueJobsTable.tsx, packages/twenty-front/src/modules/settings/admin-panel/health-status/hooks/useDeleteJobs.ts, and all confirmation modal components with pluralization issues
Context used (4)
- Rule from
dashboard- Use lingui for all user-facing strings to make them translatable, including field type labels and UI... (source) - Context from
dashboard- Follow the convention of having only one utility function per exported file in the project. (source) - Context from
dashboard- UseuseEffectinEffectComponentor hooks only. Move effects to dedicated components when necess... (source) - Context from
dashboard- Ensure that type assertions are safe and consider using type guards instead of direct assertions. (source)
27 files reviewed, 31 comments
.../twenty-front/src/modules/settings/admin-panel/health-status/graphql/queries/getQueueJobs.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/core-modules/admin-panel/dtos/queue-job.dto.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/settings/admin-panel/health-status/hooks/useDeleteJobs.ts
Outdated
Show resolved
Hide resolved
packages/twenty-front/src/modules/settings/admin-panel/health-status/hooks/useDeleteJobs.ts
Outdated
Show resolved
Hide resolved
| this.logger.log(`Retrying all failed jobs in queue ${queueName}`); | ||
| await queue.retryJobs({ state: 'failed' }); | ||
|
|
||
| return -1; |
There was a problem hiding this comment.
style: Returning -1 as a magic number to indicate 'all jobs retried' makes the API unclear. Consider returning the actual count by checking queue stats after retrying, or documenting this behavior explicitly in the return type.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel-queue.service.ts
Line: 128:128
Comment:
**style:** Returning -1 as a magic number to indicate 'all jobs retried' makes the API unclear. Consider returning the actual count by checking queue stats after retrying, or documenting this behavior explicitly in the return type.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Good enough for now imo
| private getJobState(job: Job): JobState { | ||
| if (job.finishedOn) { | ||
| return job.failedReason ? JobState.failed : JobState.completed; | ||
| } | ||
| if (job.processedOn) { | ||
| return JobState.active; | ||
| } | ||
| if (job.delay) { | ||
| return JobState.delayed; | ||
| } | ||
|
|
||
| return JobState.waiting; |
There was a problem hiding this comment.
logic: Job state determination logic has a potential race condition and doesn't match the actual job state from BullMQ. Jobs that are 'paused' will be incorrectly classified as 'waiting'. Use await job.getState() instead for accurate state information.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel-queue.service.ts
Line: 210:221
Comment:
**logic:** Job state determination logic has a potential race condition and doesn't match the actual job state from BullMQ. Jobs that are 'paused' will be incorrectly classified as 'waiting'. Use `await job.getState()` instead for accurate state information.
How can I resolve this? If you propose a fix, please make it concise.
...nty-front/src/modules/settings/admin-panel/health-status/components/JobDetailsExpandable.tsx
Outdated
Show resolved
Hide resolved
.../modules/settings/admin-panel/health-status/components/SettingsAdminJobDetailsExpandable.tsx
Show resolved
Hide resolved
| <StyledSection> | ||
| <StyledSectionTitle>{t`Logs`}</StyledSectionTitle> | ||
| {job.logs.map((log, index) => ( | ||
| <StyledLogEntry key={index}>{log}</StyledLogEntry> |
There was a problem hiding this comment.
style: Using array index as key can cause issues if log order changes. Consider using a unique identifier if available. Are logs guaranteed to be immutable and in fixed order?
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/settings/admin-panel/health-status/components/JobDetailsExpandable.tsx
Line: 133:133
Comment:
**style:** Using array index as key can cause issues if log order changes. Consider using a unique identifier if available. Are logs guaranteed to be immutable and in fixed order?
How can I resolve this? If you propose a fix, please make it concise.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:20536 This environment will automatically shut down when the PR is closed or after 5 hours. |
FelixMalfait
force-pushed
the
fix-worker-health-redis-connection
branch
from
b8d1941 to
d921ff2
Compare
...s/settings/admin-panel/health-status/components/SettingsAdminDeleteJobsConfirmationModal.tsx
Show resolved
Hide resolved
| padding: ${({ theme }) => theme.spacing(2)}; | ||
| `; | ||
|
|
||
| export const JobDetailsExpandable = ({ |
There was a problem hiding this comment.
polish: would be great to add "AdminPanel" in all these components naming to ease devXP (search in IDE is made through component is through the all codebase)
| const hasStacktrace = job.stackTrace && job.stackTrace.length > 0; | ||
| const hasFailedReason = job.failedReason; | ||
|
|
||
| const isAnyNode = () => true; |
There was a problem hiding this comment.
change: what's this? :p
|
|
||
| {hasStacktrace && job.stackTrace && ( | ||
| <StyledSection> | ||
| <StyledSectionTitle>{t`Stack Trace`}</StyledSectionTitle> |
There was a problem hiding this comment.
question/change: I'm surprised we don't have these styling already there and ready for re-use in Settings
.../modules/settings/admin-panel/health-status/components/SettingsAdminJobDetailsExpandable.tsx
Outdated
Show resolved
Hide resolved
| }; | ||
|
|
||
| const JOB_STATE_COLORS: Record<JobState, TagColor> = { | ||
| [JobState.completed]: 'green', |
There was a problem hiding this comment.
change: shouldn't all gql enums be UPPER_CASE per our latest discussions?
| gap: ${({ theme }) => theme.spacing(2)}; | ||
| `; | ||
|
|
||
| const StyledAttemptBadge = styled.span` |
There was a problem hiding this comment.
polish: can't we re-use UI component here
| white-space: nowrap; | ||
| `; | ||
|
|
||
| export const JobStateBadge = ({ |
There was a problem hiding this comment.
change: naming: this component seems to be containing Two badges + not only related to State
|
|
||
| /** Job state in the queue */ | ||
| export enum JobState { | ||
| active = 'active', |
|
|
||
| export const QueueJobsTable = ({ | ||
| queueName, | ||
| onRetentionConfigLoaded, |
There was a problem hiding this comment.
polish: see comment above, does this has to be a callback?
| fetchPolicy: 'network-only', | ||
| }); | ||
|
|
||
| const { retryJobs, isRetrying } = useRetryJobs(queueName, () => { |
There was a problem hiding this comment.
polish: this API seems complex for no reason: I expect retryJobs not to be reactive so I don't actually need to passe the queueName or the onSuccess in hook params (queueName, onSuccess) but I could either pass them in retryJobs function exposed by the hook itself.
Again, doesn't harm here as code is simple, so LGTM!
| onRetentionConfigLoaded !== undefined; | ||
|
|
||
| if (shouldPassConfig) { | ||
| onRetentionConfigLoaded(data.getQueueJobs.retentionConfig); |
There was a problem hiding this comment.
question: really not a fan of this, why is it needed?
| const someJobsSelected = | ||
| jobs.some((job) => selectedJobIds.has(job.id)) && !allJobsSelected; | ||
|
|
||
| // Check if all selected jobs are failed (for showing retry button) |
There was a problem hiding this comment.
change: I feel we start adding comments because this component is too big and it's doing too much (comments saying trivial things is still a code smell to me) let's break it into subcomponents, it should be small enough and "well-named" enough so we don't need to add these comments.
- Here we could have:
const showRetryButton = selectedJobs.length > 0 &&
selectedJobs.every((job) => job.state === 'failed');
| }; | ||
|
|
||
| const formatTimestampRelative = (timestamp?: number | null) => { | ||
| if (!timestamp) return '-'; |
| }; | ||
|
|
||
| const handleToggleJob = (event: React.MouseEvent, jobId: string) => { | ||
| event.stopPropagation(); |
There was a problem hiding this comment.
polish: inconsitent, sometimes the stopPropagation is made in the eventHandler in JSX below
| padding: ${({ theme }) => theme.spacing(2)}; | ||
| `; | ||
|
|
||
| const StyledTableCell = styled(TableCell)` |
There was a problem hiding this comment.
polish: Usually we avoid styling UI components like this as they should already implement this behavior in their API (this benefit the whole codebase)
Here we could imagine adding a maxWidth props that would add this behavior
| count | ||
| totalCount | ||
| hasMore | ||
| retentionConfig { |
There was a problem hiding this comment.
note: a bit hacky but does not harm that much
| count | ||
| totalCount | ||
| hasMore | ||
| retentionConfig { |
There was a problem hiding this comment.
because of this retentionConfig, I would tend to name the gql field getQueueState rather than getQueueJobs
| } | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Unknown error'; |
There was a problem hiding this comment.
question: should we translate this or use userFriendlyMessage?
| } | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Unknown error'; |
| <H2Title title={t`Jobs`} description={queueDescription} /> | ||
| <QueueJobsTable | ||
| queueName={queueName} | ||
| onRetentionConfigLoaded={setRetentionConfig} |
There was a problem hiding this comment.
polish: not great pattern, I would use a recoilState instead here. As components are simple that's fine but this won't play well with component re-work (if we decide to split the QueueJobsTable for instance, as we will need to drilldown)
There was a problem hiding this comment.
seeing this + the getQueueJobs returning the config ==> I wonder if we should just make another query that would return the retention config and calling it in the parent component
| } from 'src/engine/core-modules/admin-panel/enums/job-state.enum'; | ||
|
|
||
| @ObjectType() | ||
| export class QueueJob { |
There was a problem hiding this comment.
@ObjectType('QueueJob')
QueueJobDTO
(same for other files)
| // Create a GraphQL enum that matches BullMQ's JobState | ||
| // This is only used for GraphQL schema generation | ||
| export enum JobStateEnum { | ||
| completed = 'completed', |
There was a problem hiding this comment.
Uppercase and we should convert it if needed
packages/twenty-server/src/engine/core-modules/admin-panel/admin-panel-queue.service.ts
Outdated
Show resolved
Hide resolved
| this.logger.error( | ||
| `Error getting jobs for queue ${queueName}: ${error.message}`, | ||
| ); | ||
| throw error; |
There was a problem hiding this comment.
should throw CustomException with userFriendlyMessage
| await job.remove(); | ||
| deletedCount++; | ||
| } else { | ||
| this.logger.warn( |
There was a problem hiding this comment.
question: do these loggers actually help?
| exception?: { | ||
| code?: string; | ||
| stacktrace?: ReadonlyArray<string>; | ||
| stackTrace?: ReadonlyArray<string>; |
There was a problem hiding this comment.
question: is this only used by our code? or is it taken / used by an external library?
- Renamed 13 components in health-status/components folder with SettingsAdmin prefix - Updated all imports and usages across the codebase - Fixed prettier formatting issues
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[log] [log] ✖ Type AgentHandoffDTO was removed GraphQL Metadata Schema ChangesGraphQL Metadata Schema Changes[log] [log] ✖ Type AgentHandoffDTO was removed
|
… truth - Replace ResolverArgsType enum with const reference to RESOLVER_METHOD_NAMES - Remove OPERATION_NAME_TO_RESOLVER_ARGS_TYPE_MAP (no longer needed) - Pass operation names directly to queryRunnerArgsFactory - Fixes broken hooks that were using camelCase names but enum had SCREAMING_SNAKE_CASE values - This resolves phone field transformation, validation, and rich text test failures
|
Will address comments in another PR as I want to do more refactor and this one is already big |
2 participants
Adds a comprehensive queue management interface to the admin panel for viewing and managing background jobs.
Features:
Changes: