Skip to content

Conversation

@wilsonrivera
Copy link
Contributor

@wilsonrivera wilsonrivera commented Dec 16, 2025

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

    • Automated cleanup that identifies and queues inactive single-member organizations for deletion.
    • Notification workflow that emails admins when an organization is scheduled for deletion, with configurable delay.
  • Refactor

    • Organization deletion flow now runs within a single transactional boundary and supports per-request deletion delay.
    • Deletion queuing logic centralized and wired into background workers.
  • Style

    • Standardized worker log payload keys for consistent observability.
  • Tests

    • Test setup exposes the new notification queue.

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
DB cleanup script
controlplane/src/bin/db-cleanup.ts
New CLI: connects to Postgres (optional TLS) and Redis, queries single-member inactive organizations, buckets results, processes buckets in parallel transactions, checks audit logs, enqueues deletions via DeleteOrganizationQueue, and schedules notification jobs via NotifyOrganizationDeletionQueuedQueue with computed deletesAt.
Notify deletion notification
controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts
New exported input interface, queue class NotifyOrganizationDeletionQueuedQueue, and createNotifyOrganizationDeletionQueuedWorker factory. Worker loads organization and members, filters admins, formats dates, sends deletion-queued emails via Mailer, and configures retries/backoff and logging.
Build wiring / routing
controlplane/src/core/build-server.ts,
controlplane/src/core/routes.ts
Instantiates and starts notifyOrganizationDeletionQueuedQueue and its worker (mailer passed), exposes notifyOrganizationDeletionQueuedQueue in router queues, adjusts raw-body plugin options, and simplifies metrics import usage.
Repository change
controlplane/src/core/repositories/OrganizationRepository.ts
queueOrganizationDeletion signature now accepts optional deleteDelayInDays?: number and is async; updates organization and schedules deletion job using the provided delay (fallback to default) without an internal transaction wrapper.
Transactional delete flow
controlplane/src/core/bufservices/organization/deleteOrganization.ts
Delete flow runs inside a DB transaction; repositories constructed with tx; adds admin/authorization and minimum-org checks; queues deletion, writes audit log, and optionally sends deletion-notification emails within the transaction context.
Worker logging key fixes
controlplane/src/core/workers/CacheWarmerWorker.ts, .../DeactivateOrganizationWorker.ts, .../DeleteOrganizationAuditLogsWorker.ts, .../DeleteOrganizationWorker.ts, .../DeleteUserQueue.ts, .../ReactivateOrganizationWorker.ts
Cosmetic changes: stalled-event log metadata key renamed from joinId to jobId; no control-flow changes.
Tests / utilities
controlplane/test/test-util.ts
Test setup now creates NotifyOrganizationDeletionQueuedQueue and exposes it as queues.notifyOrganizationDeletionQueuedQueue.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • controlplane/src/bin/db-cleanup.ts: SQL correctness, inactivity-window and creation-date filters, bucketing logic, parallel transaction safety, and resource cleanup.
    • controlplane/src/core/workers/NotifyOrganizationDeletionQueuedWorker.ts: mailer integration, admin selection, date formatting, and job retry/backoff defaults.
    • controlplane/src/core/repositories/OrganizationRepository.ts: semantics of removing the transaction wrapper and correct usage of deleteDelayInDays fallback.
    • controlplane/src/core/bufservices/organization/deleteOrganization.ts: transaction boundaries, repository constructor signature changes, and audit log ordering relative to queuing.
    • Build/routing wiring (build-server.ts, routes.ts): ensure worker instantiation receives mailer and redis arguments and the queue is exposed correctly.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the primary change: creating a script to enqueue inactive organizations for deletion, which matches the main code additions.
✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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-ignore and simplify chunk logic.

The @ts-ignore suppresses a type error for a condition that's always false (MAX_DEGREE_OF_PARALLELISM === 1 when it's defined as 5). 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.error for 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: Use getOrganizationAdmins instead of filtering all members.

OrganizationRepository has a getOrganizationAdmins method 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 direct process.env access; pass webBaseUrl via configuration.

The worker accesses process.env.WEB_BASE_URL directly, 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: Pass webBaseUrl to the worker for consistency.

The worker uses process.env.WEB_BASE_URL directly, but opts.auth.webBaseUrl is 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

📥 Commits

Reviewing files that changed from the base of the PR and between caaf2bd and 4804e55.

📒 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 deleteDelayInDays parameter provides flexibility for different deletion workflows while maintaining backward compatibility by falling back to delayForManualOrgDeletionInDays.

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 the startOfMonth usage 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 be subDays(now, MIN_INACTIVITY_DAYS)?


49-54: The review comment is incorrect. The code properly handles Redis TLS configuration:

  1. redis.host is never undefined due to the default value process.env.REDIS_HOST || 'localhost' in get-config.ts, making the ! assertion on line 50 redundant but harmless.

  2. The tls property is correctly handled as optional. In get-config.ts, it's conditionally set to an object only when TLS environment variables are present, otherwise undefined. The RedisPluginOptions interface in redis.ts properly defines tls as optional with all its properties optional.

  3. createRedisConnections safely 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.

Copy link

@coderabbitai coderabbitai bot left a 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 orgAdmins is empty (line 87), the email will be sent with an empty receiverEmails array (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 joinId to jobId corrects a typo and ensures consistent logging across workers.

Optional: Rename the parameter for clarity

The job parameter in the stalled event callback is actually the job ID string, not a Job object. Consider renaming it to jobId for 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 joinId to jobId correctly aligns the log key with the actual value being logged and standardizes the approach across workers.

Optional: Rename parameter for clarity

The callback parameter job receives the jobId string (per BullMQ's stalled event signature), not a Job object. Consider renaming it to jobId for 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 joinId to jobId is 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4804e55 and a0c978a.

📒 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 joinId to jobId correctly 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 joinId to jobId correctly 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 jobId for 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.

Copy link

@coderabbitai coderabbitai bot left a 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:

  1. Refactor orgRepo.queueOrganizationDeletion to accept and use the transaction context (tx) passed from the outer transaction, avoiding nested transactions
  2. 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 redisQueue and redisWorker, but only redisQueue is used to initialize the queue instances. The redisWorker connection 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

📥 Commits

Reviewing files that changed from the base of the PR and between a0c978a and e82d01a.

📒 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
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

❌ Patch coverage is 21.53846% with 306 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.02%. Comparing base (e15b640) to head (2394618).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
controlplane/src/bin/db-cleanup.ts 0.00% 159 Missing and 1 partial ⚠️
...ore/bufservices/organization/deleteOrganization.ts 0.00% 89 Missing ⚠️
.../workers/NotifyOrganizationDeletionQueuedWorker.ts 50.48% 51 Missing ⚠️
controlplane/src/core/workers/CacheWarmerWorker.ts 0.00% 1 Missing ⚠️
...e/src/core/workers/DeactivateOrganizationWorker.ts 0.00% 1 Missing ⚠️
.../core/workers/DeleteOrganizationAuditLogsWorker.ts 0.00% 1 Missing ⚠️
...plane/src/core/workers/DeleteOrganizationWorker.ts 0.00% 1 Missing ⚠️
controlplane/src/core/workers/DeleteUserQueue.ts 0.00% 1 Missing ⚠️
...e/src/core/workers/ReactivateOrganizationWorker.ts 0.00% 1 Missing ⚠️

❌ 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     
Files with missing lines Coverage Δ
controlplane/src/core/build-server.ts 76.40% <100.00%> (ø)
...ne/src/core/repositories/OrganizationRepository.ts 77.16% <100.00%> (ø)
controlplane/src/core/routes.ts 100.00% <ø> (ø)
controlplane/src/core/workers/CacheWarmerWorker.ts 0.00% <0.00%> (ø)
...e/src/core/workers/DeactivateOrganizationWorker.ts 54.73% <0.00%> (ø)
.../core/workers/DeleteOrganizationAuditLogsWorker.ts 87.50% <0.00%> (ø)
...plane/src/core/workers/DeleteOrganizationWorker.ts 90.26% <0.00%> (ø)
controlplane/src/core/workers/DeleteUserQueue.ts 47.52% <0.00%> (ø)
...e/src/core/workers/ReactivateOrganizationWorker.ts 80.89% <0.00%> (ø)
.../workers/NotifyOrganizationDeletionQueuedWorker.ts 50.48% <50.48%> (ø)
... and 2 more

... and 514 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a 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 userId field is selected but never used in processChunkOfOrganizations. 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

📥 Commits

Reviewing files that changed from the base of the PR and between e82d01a and 67c0a22.

📒 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants