-
Notifications
You must be signed in to change notification settings - Fork 201
feat: create script to enqueue inactive organizations for deletion #2418
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.
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a db-cleanup CLI to find and enqueue inactive single-member organizations for deletion, introduces a NotifyOrganizationDeletionQueued queue/worker and routes wiring, updates OrganizationRepository.queueOrganizationDeletion to accept per-request delay, converts deleteOrganization flow to run in a DB transaction, and updates several worker log keys. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (5)
controlplane/src/bin/db-cleanup.ts (2)
88-101: Remove@ts-ignoreand simplify chunk logic.The
@ts-ignoresuppresses a type error for a condition that's always false (MAX_DEGREE_OF_PARALLELISM === 1when it's defined as5). Consider removing the special case or making it configurable if the single-threaded path is needed.function chunkArray<T>(data: T[]): T[][] { - // @ts-ignore - if (MAX_DEGREE_OF_PARALLELISM === 1) { - return [data]; - } - const chunks: T[][] = []; const organizationsPerChunk = Math.ceil(ORGANIZATIONS_PER_BUCKET / MAX_DEGREE_OF_PARALLELISM); for (let i = 0; i < data.length; i += organizationsPerChunk) { chunks.push(data.slice(i, i + organizationsPerChunk)); } - return chunks; }
117-117: Consider using the pino logger consistently.A pino logger is created on line 62, but the script uses
console.log/console.errorfor output (lines 117, 134, 138, 141, 156, 189). For consistency with the rest of the codebase and better structured logging, consider using the pino logger throughout.Also applies to: 138-138, 141-141, 156-156, 189-189
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (2)
86-87: UsegetOrganizationAdminsinstead of filtering all members.
OrganizationRepositoryhas agetOrganizationAdminsmethod that directly returns admin members. This avoids loading RBAC data for all members just to filter.- const organizationMembers = await orgRepo.getMembers({ organizationID: org.id }); - const orgAdmins = organizationMembers.filter((m) => m.rbac.isOrganizationAdmin); + const orgAdmins = await orgRepo.getOrganizationAdmins({ organizationID: org.id });
100-100: Avoid directprocess.envaccess; passwebBaseUrlvia configuration.The worker accesses
process.env.WEB_BASE_URLdirectly, which is inconsistent with other workers that receive configuration through their input options. This also makes testing harder.+// In createNotifyOrganizationDeletionQueuedWorker input: + webBaseUrl: string; // In handler: - restoreLink: `${process.env.WEB_BASE_URL}/${org.slug}/settings`, + restoreLink: `${this.input.webBaseUrl}/${org.slug}/settings`,controlplane/src/core/build-server.ts (1)
406-411: PasswebBaseUrlto the worker for consistency.The worker uses
process.env.WEB_BASE_URLdirectly, butopts.auth.webBaseUrlis available here. Pass it to maintain consistency with how other components receive configuration.createNotifyOrganizationDeletionQueuedWorker({ redisConnection: fastify.redisForWorker, db: fastify.db, logger, mailer: mailerClient, + webBaseUrl: opts.auth.webBaseUrl, }),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
controlplane/src/bin/db-cleanup.ts(1 hunks)controlplane/src/core/build-server.ts(4 hunks)controlplane/src/core/repositories/OrganizationRepository.ts(2 hunks)controlplane/src/core/routes.ts(2 hunks)controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/src/bin/db-cleanup.ts
🧬 Code graph analysis (5)
controlplane/src/core/repositories/OrganizationRepository.ts (1)
controlplane/src/core/constants.ts (1)
delayForManualOrgDeletionInDays(10-10)
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (3)
controlplane/src/core/workers/Worker.ts (2)
IQueue(3-7)IWorker(9-11)controlplane/src/core/services/Mailer.ts (1)
Mailer(13-101)controlplane/src/core/repositories/OrganizationRepository.ts (1)
OrganizationRepository(50-1681)
controlplane/src/bin/db-cleanup.ts (5)
controlplane/src/core/plugins/redis.ts (1)
createRedisConnections(29-86)controlplane/src/core/workers/DeleteOrganizationWorker.ts (1)
DeleteOrganizationQueue(20-62)controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (1)
NotifyOrganizationDeletionQueuedQueue(18-60)controlplane/src/db/schema.ts (2)
organizations(1266-1289)auditLogs(1936-1972)controlplane/src/core/repositories/OrganizationRepository.ts (1)
OrganizationRepository(50-1681)
controlplane/src/core/routes.ts (1)
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (1)
NotifyOrganizationDeletionQueuedQueue(18-60)
controlplane/src/core/build-server.ts (1)
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (2)
NotifyOrganizationDeletionQueuedQueue(18-60)createNotifyOrganizationDeletionQueuedWorker(112-134)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_test
- GitHub Check: build_push_image
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
controlplane/src/core/repositories/OrganizationRepository.ts (1)
942-970: LGTM! Clean extension of deletion scheduling.The optional
deleteDelayInDaysparameter provides flexibility for different deletion workflows while maintaining backward compatibility by falling back todelayForManualOrgDeletionInDays.controlplane/src/core/routes.ts (1)
24-24: LGTM! Consistent queue wiring.The new queue follows the established pattern for queue registration in RouterOptions.
Also applies to: 52-52
controlplane/src/bin/db-cleanup.ts (2)
114-116: Clarify thestartOfMonthusage for inactivity threshold.Using
startOfMonth(subDays(now, MIN_INACTIVITY_DAYS))creates a threshold that varies depending on the current day of the month. For example, running on Dec 16 vs Dec 1 yields different threshold dates. Was this intentional for batch alignment, or should it simply besubDays(now, MIN_INACTIVITY_DAYS)?
49-54: The review comment is incorrect. The code properly handles Redis TLS configuration:
redis.hostis never undefined due to the default valueprocess.env.REDIS_HOST || 'localhost'inget-config.ts, making the!assertion on line 50 redundant but harmless.The
tlsproperty is correctly handled as optional. Inget-config.ts, it's conditionally set to an object only when TLS environment variables are present, otherwiseundefined. TheRedisPluginOptionsinterface inredis.tsproperly definestlsas optional with all its properties optional.
createRedisConnectionssafely checks each TLS property before use (lines 45, 49, 54 in redis.ts), validating file paths before reading.No validation is needed because TLS configuration is properly optional and defensively implemented.
Likely an incorrect or invalid review comment.
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (1)
18-60: LGTM! Queue implementation follows established patterns.The queue configuration with exponential backoff and job retention is consistent with other queues in the codebase.
controlplane/src/core/build-server.ts (1)
401-412: LGTM! Queue and worker wiring follows established patterns.The new notification queue and worker are registered consistently with other queues in the build server, using the same connection patterns and passing the mailer client appropriately.
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (2)
76-78: Throwing error when mailer not configured causes unnecessary retries.When the mailer is not configured, throwing an error triggers the retry mechanism (6 attempts with exponential backoff). This will cause the job to fail repeatedly for approximately 1.3 hours before giving up, wasting resources.
Consider logging a warning and returning early instead:
if (!this.input.mailer) { - throw new Error('Mailer service not configured'); + this.input.logger.warn('Mailer service not configured, skipping notification'); + return; }
87-101: Handle case where organization has no admins.If
orgAdminsis empty (line 87), the email will be sent with an emptyreceiverEmailsarray (line 95). Consider adding a guard to skip the email or log a warning:const orgAdmins = organizationMembers.filter((m) => m.rbac.isOrganizationAdmin); + + if (orgAdmins.length === 0) { + this.input.logger.warn({ organizationId: org.id }, 'No admins found for organization, skipping notification'); + return; + } const intl = Intl.DateTimeFormat(undefined, {
🧹 Nitpick comments (3)
controlplane/src/core/workers/ReactivateOrganizationWorker.ts (1)
115-116: LGTM! Typo fix aligns logging with other workers.The change from
joinIdtojobIdcorrects a typo and ensures consistent logging across workers.Optional: Rename the parameter for clarity
The
jobparameter in the stalled event callback is actually the job ID string, not a Job object. Consider renaming it tojobIdfor clarity:- worker.on('stalled', (job) => { - log.warn({ jobId: job }, `Job stalled`); + worker.on('stalled', (jobId) => { + log.warn({ jobId }, `Job stalled`); });Based on learnings, this change aligns with similar updates across other workers in the PR.
controlplane/src/core/workers/DeactivateOrganizationWorker.ts (1)
127-129: LGTM! Key name fix improves consistency.The change from
joinIdtojobIdcorrectly aligns the log key with the actual value being logged and standardizes the approach across workers.Optional: Rename parameter for clarity
The callback parameter
jobreceives the jobId string (per BullMQ's stalled event signature), not a Job object. Consider renaming it tojobIdfor clarity:- worker.on('stalled', (job) => { - log.warn({ jobId: job }, `Job stalled`); + worker.on('stalled', (jobId) => { + log.warn({ jobId }, `Job stalled`); });controlplane/src/core/workers/CacheWarmerWorker.ts (1)
133-135: Good typo fix; consider logging just the job ID for consistency.The key name correction from
joinIdtojobIdis excellent and aligns with the broader pattern across workers.For consistency with the error handler on line 112 (which logs
jobId: job.id), consider logging just the job ID rather than the entire job object. This keeps logs concise and matches the established pattern in this file.🔎 Optional refinement for consistency:
- log.warn({ jobId: job }, `Job stalled`); + log.warn({ jobId: job.id }, `Job stalled`);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
controlplane/src/bin/db-cleanup.ts(1 hunks)controlplane/src/core/bufservices/organization/deleteOrganization.ts(1 hunks)controlplane/src/core/repositories/OrganizationRepository.ts(1 hunks)controlplane/src/core/workers/CacheWarmerWorker.ts(1 hunks)controlplane/src/core/workers/DeactivateOrganizationWorker.ts(1 hunks)controlplane/src/core/workers/DeleteOrganizationAuditLogsWorker.ts(1 hunks)controlplane/src/core/workers/DeleteOrganizationWorker.ts(1 hunks)controlplane/src/core/workers/DeleteUserQueue.ts(1 hunks)controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts(1 hunks)controlplane/src/core/workers/ReactivateOrganizationWorker.ts(1 hunks)controlplane/test/test-util.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- controlplane/src/core/workers/DeleteOrganizationAuditLogsWorker.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- controlplane/src/bin/db-cleanup.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/src/core/bufservices/organization/deleteOrganization.ts
📚 Learning: 2025-07-01T13:53:54.146Z
Learnt from: wilsonrivera
Repo: wundergraph/cosmo PR: 1919
File: controlplane/src/core/repositories/OrganizationGroupRepository.ts:193-224
Timestamp: 2025-07-01T13:53:54.146Z
Learning: In the Cosmo codebase, database transactions are typically managed at the service layer (e.g., in buf services like deleteOrganizationGroup.ts), where repositories are instantiated with the transaction handle and all operations within those repositories are automatically part of the same transaction.
Applied to files:
controlplane/src/core/bufservices/organization/deleteOrganization.ts
🧬 Code graph analysis (2)
controlplane/src/core/repositories/OrganizationRepository.ts (2)
controlplane/src/core/workers/DeleteOrganizationWorker.ts (1)
DeleteOrganizationQueue(20-62)controlplane/src/core/constants.ts (1)
delayForManualOrgDeletionInDays(10-10)
controlplane/test/test-util.ts (1)
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (1)
NotifyOrganizationDeletionQueuedQueue(18-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
controlplane/src/core/workers/DeleteUserQueue.ts (1)
143-143: LGTM: Logging key corrected.The change from
joinIdtojobIdcorrectly aligns the log metadata with the actual job identifier, consistent with similar corrections across other workers in this PR.controlplane/src/core/workers/DeleteOrganizationWorker.ts (1)
154-154: LGTM: Logging key corrected.The change from
joinIdtojobIdcorrectly aligns the log metadata with the actual job identifier, matching the pattern applied across other workers.controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts (1)
128-128: LGTM: Logging key is correct.The stalled event handler correctly uses
jobIdfor the log metadata.controlplane/src/core/bufservices/organization/deleteOrganization.ts (1)
28-140: Transaction scope correctly established.The transaction wrapper correctly ensures all repository operations execute atomically. The repository instantiation with the transaction handle (lines 29-31) follows the established pattern for this codebase.
However, note that
queueOrganizationDeletion(lines 100-104) performs a queue operation (addJob) that cannot be rolled back if a subsequent operation within this transaction fails. This is acceptable for notification-type queues but worth documenting.Based on learnings, the transactional approach at the service layer is appropriate for coordinating multiple repository operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
controlplane/src/bin/db-cleanup.ts (1)
149-161: Critical: Nested transaction and Redis atomicity issues remain unaddressed.This is the same issue previously flagged: the outer transaction at line 151 wraps
processChunkOfOrganizations, but within that function,orgRepo.queueOrganizationDeletion(line 199) starts its own transaction, creating nested transactions. Additionally, the Redis notification job (lines 207-211) is enqueued outside the DB transaction boundary, risking inconsistency if the transaction fails after the job is enqueued.Required fixes:
- Refactor
orgRepo.queueOrganizationDeletionto accept and use the transaction context (tx) passed from the outer transaction, avoiding nested transactions- Move Redis job enqueueing inside the repository method or implement an outbox pattern to ensure the notification is only sent if the DB transaction commits successfully
Run the following script to verify the transaction handling in
OrganizationRepository.queueOrganizationDeletion:#!/bin/bash # Check if queueOrganizationDeletion starts its own transaction ast-grep --pattern $'queueOrganizationDeletion($$$) { $$$ transaction($$$) $$$ }'
🧹 Nitpick comments (1)
controlplane/src/bin/db-cleanup.ts (1)
56-72: Consider removing unused redisWorker connection.The script connects and pings both
redisQueueandredisWorker, but onlyredisQueueis used to initialize the queue instances. TheredisWorkerconnection appears unnecessary for this script.🔎 Proposed simplification
- const { redisQueue, redisWorker } = await createRedisConnections({ + const { redisQueue } = await createRedisConnections({ host: redis.host!, port: Number(redis.port), password: redis.password, tls: redis.tls, }); await redisQueue.connect(); - await redisWorker.connect(); - await redisWorker.ping(); await redisQueue.ping(); // ... rest of code ... } finally { redisQueue.disconnect(); - redisWorker.disconnect();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
controlplane/src/bin/db-cleanup.ts(1 hunks)controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts(1 hunks)controlplane/test/test-util.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- controlplane/test/test-util.ts
- controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/src/bin/db-cleanup.ts
📚 Learning: 2025-07-01T13:53:54.146Z
Learnt from: wilsonrivera
Repo: wundergraph/cosmo PR: 1919
File: controlplane/src/core/repositories/OrganizationGroupRepository.ts:193-224
Timestamp: 2025-07-01T13:53:54.146Z
Learning: In the Cosmo codebase, database transactions are typically managed at the service layer (e.g., in buf services like deleteOrganizationGroup.ts), where repositories are instantiated with the transaction handle and all operations within those repositories are automatically part of the same transaction.
Applied to files:
controlplane/src/bin/db-cleanup.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build_push_image
- GitHub Check: build_test
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
controlplane/src/bin/db-cleanup.ts (2)
128-135: LGTM! Proper filtering for inactive organizations.The WHERE clause correctly filters for organizations that:
- Are not already queued for deletion
- Are not deactivated (avoiding duplicates)
- Were created before the inactivity threshold
- Have no billing plan or are on the developer plan
This addresses the previous review concerns about excluding deactivated organizations and checking billing plans.
184-194: LGTM! Proper inactivity verification.The audit log check correctly verifies whether an organization has had any activity within the inactivity window. Organizations with recent activity are appropriately skipped, ensuring only truly inactive organizations are enqueued for deletion.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (21.53%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2418 +/- ##
==========================================
+ Coverage 61.35% 62.02% +0.66%
==========================================
Files 229 297 +68
Lines 23814 41429 +17615
Branches 0 4197 +4197
==========================================
+ Hits 14612 25695 +11083
- Misses 7970 15713 +7743
+ Partials 1232 21 -1211
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
controlplane/src/bin/db-cleanup.ts (2)
122-122: Remove unused userId field from query.The
userIdfield is selected but never used inprocessChunkOfOrganizations. Consider removing it to slightly reduce query overhead.🔎 Proposed fix
.select({ id: schema.organizations.id, slug: schema.organizations.slug, - userId: schema.organizations.createdBy, plan: schema.organizationBilling.plan, })And update the type signature on line 173:
- organizations: { id: string; slug: string; userId: string | null }[]; + organizations: { id: string; slug: string }[];
182-182: Reuse the parent logger for consistency.Creating a new
pino()logger here instead of reusing the logger from line 62 leads to inconsistent logging across the script. The queue constructors (lines 67-68) receive the parent logger, but the repository receives a separate instance.🔎 Proposed fix
Pass the logger as a parameter to
processChunkOfOrganizations:async function processChunkOfOrganizations({ organizations, db, inactivityThreshold, deleteOrganizationQueue, notifyOrganizationDeletionQueuedQueue, + logger, }: { organizations: { id: string; slug: string; userId: string | null }[]; db: PostgresJsDatabase<typeof schema>; inactivityThreshold: Date; deleteOrganizationQueue: DeleteOrganizationQueue; notifyOrganizationDeletionQueuedQueue: NotifyOrganizationDeletionQueuedQueue; + logger: pino.Logger; }) { const queuedAt = new Date(); const deletesAt = addDays(queuedAt, DELAY_FOR_ORG_DELETION_IN_DAYS); - const orgRepo = new OrganizationRepository(pino(), db, undefined); + const orgRepo = new OrganizationRepository(logger, db, undefined);And update the call site around line 152:
return processChunkOfOrganizations({ organizations: chunk, db: tx, inactivityThreshold, deleteOrganizationQueue, notifyOrganizationDeletionQueuedQueue, + logger, });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
controlplane/src/bin/db-cleanup.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-29T10:28:04.846Z
Learnt from: JivusAyrus
Repo: wundergraph/cosmo PR: 2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1749-1751
Timestamp: 2025-08-29T10:28:04.846Z
Learning: In the controlplane codebase, authentication and authorization checks (including organization scoping) are handled at the service layer in files like unlinkSubgraph.ts before calling repository methods. Repository methods like unlinkSubgraph() in SubgraphRepository.ts can focus purely on data operations without redundant security checks.
Applied to files:
controlplane/src/bin/db-cleanup.ts
📚 Learning: 2025-07-01T13:53:54.146Z
Learnt from: wilsonrivera
Repo: wundergraph/cosmo PR: 1919
File: controlplane/src/core/repositories/OrganizationGroupRepository.ts:193-224
Timestamp: 2025-07-01T13:53:54.146Z
Learning: In the Cosmo codebase, database transactions are typically managed at the service layer (e.g., in buf services like deleteOrganizationGroup.ts), where repositories are instantiated with the transaction handle and all operations within those repositories are automatically part of the same transaction.
Applied to files:
controlplane/src/bin/db-cleanup.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: build_test
- GitHub Check: build_push_image
🔇 Additional comments (2)
controlplane/src/bin/db-cleanup.ts (2)
17-28: Well-defined configuration constants.The constants are clearly documented and provide sensible defaults:
- 3-month inactivity threshold gives adequate time before considering deletion
- 7-day deletion delay provides a reasonable recovery window
- Parallelism and batch size are appropriate for a background cleanup script
128-138: LGTM! Query correctly filters target organizations.The selection criteria are well-implemented:
- Excludes already-queued and deactivated organizations (avoiding duplicates)
- Properly scopes to free-tier plans (developer or null)
- Correctly identifies single-member organizations via HAVING clause
This addresses the plan-check concern and prevents re-queueing deactivated organizations.
The goal of this PR is to introduce a script to enqueue the deletion of inactive organizations and send a message so owners can prevent the deletion before the time runs out.
Currently only organizations that have been inactive for more than 3 months and only have a single member are considered for deletion.
Summary by CodeRabbit
New Features
Refactor
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist