Skip to content

Conversation

@knutties
Copy link
Collaborator

@knutties knutties commented Feb 1, 2026

Problem

The org cleanup script was hard-coded to cleanup only orgs starting with org% and single workspaces could not be removed cleanly.

Solution

This PR parameterizes the sql / shell script to enable any org / workspace deletion completely.

Summary by CodeRabbit

  • Tests
    • Enhanced cleanup scripts with dual-mode operation: selectively clean specific workspaces or perform full organization cleanup
    • Added organization pattern filtering for more granular resource management control
    • Improved cleanup reporting with mode-specific logging and detailed status updates

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

@knutties knutties requested a review from a team as a code owner February 1, 2026 15:04
@semanticdiff-com
Copy link

semanticdiff-com bot commented Feb 1, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  tests/package.json  88% smaller
  tests/cleanup.sql Unsupported file format
  tests/test-with-cleanup.sh Unsupported file format

@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • 🔍 Trigger a full review

Walkthrough

The PR enhances the test cleanup infrastructure by introducing organizational pattern filtering and workspace-specific cleanup modes. The SQL cleanup script now supports two conditional paths: specific workspace cleanup (when workspace_name is provided) and full organization cleanup (default). The shell wrapper was refactored to parse --org-pattern and --workspace arguments and pass them as variables to the SQL script.

Changes

Cohort / File(s) Summary
SQL Cleanup Logic
tests/cleanup.sql
Replaced single cleanup loop with dual-mode flow: specific workspace cleanup mode (filters orgs by pattern, drops only matching workspaces) and full organization cleanup mode (original behavior). Added mode-specific logging and enhanced summary reporting with conditional organization deletion counter.
Script Configuration
tests/package.json
Updated cleanup script invocation with --org-pattern "org%" filter to scope resource cleanup.
Shell Wrapper
tests/test-with-cleanup.sh
Refactored argument parsing from legacy for-loop to while-loop supporting --org-pattern and --workspace options. Added environment variable loading from .env file and dynamic DATABASE_URL construction from individual variables. Updated psql invocation to pass org pattern and workspace name as variables to SQL script.

Sequence Diagram(s)

sequenceDiagram
    participant Shell as Shell Script<br/>(test-with-cleanup.sh)
    participant Env as Environment<br/>Variables
    participant SQL as SQL Script<br/>(cleanup.sql)
    participant DB as PostgreSQL<br/>Database

    Shell->>Env: Load .env file
    Shell->>Shell: Parse --org-pattern & --workspace args
    Shell->>Shell: Construct DATABASE_URL (if needed)
    
    alt Specific Workspace Mode
        Shell->>SQL: Pass org_pattern & workspace_name
        SQL->>DB: Filter orgs by org_pattern
        SQL->>DB: Find workspaces matching workspace_name
        SQL->>DB: Drop specific workspace schemas
        SQL->>DB: Delete specific workspace records
    else Full Organization Mode
        Shell->>SQL: Pass org_pattern (workspace_name empty)
        SQL->>DB: Filter orgs by org_pattern
        SQL->>DB: Drop all workspace schemas for org
        SQL->>DB: Delete all workspace records for org
        SQL->>DB: Delete organization records
    end
    
    SQL-->>Shell: Return cleanup summary
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • ayushjain17
  • Datron

Poem

🐰 With whiskers twitching in the spring breeze,
I've crafted cleanup modes that surely please,
Org patterns filter, workspaces dance—
Two paths of cleansing, give them a chance! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'ci: support any org/workspace cleanup' directly and clearly summarizes the main change: extending cleanup scripts to handle arbitrary organizations and workspaces via parameterization.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch org-cleanup-with-args

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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: 2

🤖 Fix all issues with AI agents
In `@tests/cleanup.sql`:
- Around line 12-14: Change the current_setting calls used to initialize
v_org_pattern and v_workspace_name so they don't raise when the settings are
absent: replace current_setting('cleanup.org_pattern') and
current_setting('cleanup.workspace_name') with
current_setting('cleanup.org_pattern', true) and
current_setting('cleanup.workspace_name', true') (or wrap them in COALESCE(...,
'<default>') if you need a fallback), update the v_org_pattern and
v_workspace_name initializations accordingly, and ensure any later logic that
assumes non-NULL values checks for NULL or uses the chosen defaults.

In `@tests/test-with-cleanup.sh`:
- Around line 14-21: The --org-pattern and --workspace case branches currently
assign ORG_PATTERN="$2" and WORKSPACE_NAME="$2" without validating the next
token; update the case handlers for the --org-pattern and --workspace options to
check that "$2" is non-empty and does not begin with a dash (e.g., [[ -n "$2" &&
"${2:0:1}" != "-" ]]); if the check fails, print a clear usage/error message and
exit nonzero instead of assigning, otherwise assign the value and shift 2 as
before so ORG_PATTERN and WORKSPACE_NAME are safely populated.
🧹 Nitpick comments (1)
tests/cleanup.sql (1)

28-31: SQL injection risk: User-supplied pattern used directly in LIKE clause.

The v_org_pattern value is used directly in the WHERE id LIKE v_org_pattern clause. Since LIKE is a parameterized comparison in PL/pgSQL (not string concatenation), this is safe from SQL injection. The risk here is more about unintended data deletion if a broad pattern like % is passed.

Consider adding validation or a confirmation notice when the pattern could match many organizations (e.g., patterns starting with %).

Also applies to: 64-67

Comment on lines +12 to +14
-- Retrieve variables from session config
v_org_pattern text := current_setting('cleanup.org_pattern');
v_workspace_name text := current_setting('cleanup.workspace_name');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

current_setting() will throw an error if the setting doesn't exist.

When the session config isn't set (e.g., if set_config wasn't called), current_setting() raises an exception. Consider using current_setting('cleanup.org_pattern', true) to return NULL on missing settings instead of erroring.

🛡️ Proposed fix to handle missing settings gracefully
     -- Retrieve variables from session config
-    v_org_pattern text := current_setting('cleanup.org_pattern');
-    v_workspace_name text := current_setting('cleanup.workspace_name');
+    v_org_pattern text := current_setting('cleanup.org_pattern', true);
+    v_workspace_name text := current_setting('cleanup.workspace_name', true);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
-- Retrieve variables from session config
v_org_pattern text := current_setting('cleanup.org_pattern');
v_workspace_name text := current_setting('cleanup.workspace_name');
-- Retrieve variables from session config
v_org_pattern text := current_setting('cleanup.org_pattern', true);
v_workspace_name text := current_setting('cleanup.workspace_name', true);
🤖 Prompt for AI Agents
In `@tests/cleanup.sql` around lines 12 - 14, Change the current_setting calls
used to initialize v_org_pattern and v_workspace_name so they don't raise when
the settings are absent: replace current_setting('cleanup.org_pattern') and
current_setting('cleanup.workspace_name') with
current_setting('cleanup.org_pattern', true) and
current_setting('cleanup.workspace_name', true') (or wrap them in COALESCE(...,
'<default>') if you need a fallback), update the v_org_pattern and
v_workspace_name initializations accordingly, and ensure any later logic that
assumes non-NULL values checks for NULL or uses the chosen defaults.

@knutties knutties force-pushed the org-cleanup-with-args branch from afc7e93 to a1142b7 Compare February 2, 2026 08:45
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@knutties knutties force-pushed the org-cleanup-with-args branch from a1142b7 to 76585dd Compare February 2, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants