You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
db: drop the lowercased sandbox name index - #1613
Drops idx_sandboxes_name_lower, which #1522 added on sandboxes for the admin list's case-insensitive name search. #1523 now makes that search match IDs only, so nothing reads the index. Names are still found by the list's case-sensitive prefix.
The index costs more than it buys:
ECW/E2B creates never set a name (crates/l40_api/api/src/routes/e2b/translate.rs, name: None), and native names are optional. So it mostly indexes NULL keys.
Every non-HOT write to sandboxes, the busiest table, inserts an entry into it.
#1522's other index, idx_sandboxes_id_suffix, stays: the ID search reads it.
Migration only, as the isolation check requires:
up: DROP INDEX CONCURRENTLY IF EXISTS public.idx_sandboxes_name_lower;, a single-statement -- no-transaction file.
down: rebuilds it exactly as 20261105161136_sandboxes_name_lower_index.up.sql did (CONCURRENTLY, the same key), with that file's recovery notes, adapted to a down.
#1522's files are unchanged, so their checksums still match.
Validation
scripts/check-postgres-migrations-isolated.sh, scripts/check-migration-versions.sh and scripts/check-postgres-migrations-squawk.sh (squawk 2.69.0, 0 issues) pass against main.
On a disposable Postgres 16, space-db-migrator control-plane built at this head ran (transcript):
up from empty: 20261105161136 builds the index, then this migration drops it;
Severity: low. It removes write overhead and index storage; nothing is incorrect without it.
Risk: low. On main nothing reads the index; its only reader was #1523's previous head, which never merged.
Rollback: run this migration's down, which rebuilds the index concurrently.
Rollout
A deployment that has run #1522's migration has already built idx_sandboxes_name_lower. A deployment that runs migrations before this lands will build it too. The drop handles either case: IF EXISTS makes it a no-op where the index is gone, and CONCURRENTLY keeps writes flowing while it drops.
A deploy that carries both #1522 and this migration builds the index and then drops it. That costs one concurrent build, and is harmless.
The drop runs under the migrator session's timeouts and is not retried. It waits for every transaction holding a lock on sandboxes. If it fails, it records nothing, possibly leaving the index INVALID, and the next migrator run drops it.
No API on main reads the index, so this can merge before or after #1523 and #1524.
AI assistance: Claude Code drafted the migration and this description, and ran the checks.
The admin list's search no longer matches names (#1523), so nothing reads
idx_sandboxes_name_lower. Most sandbox rows have no name, and every
non-HOT write to the table still inserts into it. Drop it concurrently;
the down rebuilds it as its create migration did.
Automatic approval is disabled by the configured approval policy for this pull request. Review comments and requests for changes continue normally.
Commit: 514661d0f190.
No actionable comments were generated in the recent review.
Walkthrough
The migration drops public.idx_sandboxes_name_lower concurrently if it exists. Its rollback creates the index concurrently on public.sandboxes using left(lower(name), 256) with "C" collation.
Possibly related PRs
perplexityai/SPACE#1522: Creates the index that this migration drops and recreates on rollback.
Merge Risk: Minimal · up to 51466
The index removal does not affect the inspected admin search query, and no actionable merge risk remains after normal migration checks.
Security architecture risk: Low · up to 51466
The change removes a non-unique search index without changing sandbox data, permissions, or the current search contract. Rollback restores the original definition. Operational risk remains because interrupted concurrent index operations can require manual recovery, and compatibility with older deployed application versions was not established.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The direct scope is a shared control-plane table’s secondary index, not an individual tenant’s index. Any migration wait or older-query performance effect could therefore affect the shared database, although no tenant-isolation or authorization change appears in the executable SQL.
Trust Boundaries and Controls
inferred — The routed public-entrypoint signal identifies static migration DDL, not a new request handler. The changed statements have fixed database targets and contain no request-derived SQL or new grant of authority.
Resilience and Maintainability Implications
observed — The existing runner avoids automatically retrying potentially partial non-transactional migrations and deliberately leaves rollback supervised. This limits unsafe automatic repetition but leaves index/history reconciliation dependent on operator recovery after interruption.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title describes the index removal and meets the title format requirements.
Description Follows The Pr Template
The description contains all four required items: a summary, validation and results, severity and risk, and an AI assistance line that identifies the tool and its contributions.
Bug Fixes Name A Regression Test
The PR only adds database migrations to remove and restore an index. It does not fix an application behavior defect under crates/, shim/, or ui/, so the check does not apply.
Weakened Safeguards Are Justified
The new migration files explain the Squawk directives and timeout exceptions in adjacent comments. The diff does not weaken any safeguards without a specific reason.
Public-Safe Content
The diff, title, and description contain no content that matches the specified failure conditions.
Recent review info
Automatic approval policy
Commit: 514661d0f190.
Setting
Effective value
Source
Automatic approvals
Disabled
Override 4
Merge risk
Low
Override 2
Security risk
Not set
Global configuration
Matching overrides (numbered as in the resolved configuration):
Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.
Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Drops
idx_sandboxes_name_lower, which #1522 added onsandboxesfor the admin list's case-insensitive name search. #1523 now makes that search match IDs only, so nothing reads the index. Names are still found by the list's case-sensitiveprefix.The index costs more than it buys:
crates/l40_api/api/src/routes/e2b/translate.rs,name: None), and native names are optional. So it mostly indexes NULL keys.sandboxes, the busiest table, inserts an entry into it.#1522's other index,
idx_sandboxes_id_suffix, stays: the ID search reads it.Migration only, as the isolation check requires:
DROP INDEX CONCURRENTLY IF EXISTS public.idx_sandboxes_name_lower;, a single-statement-- no-transactionfile.20261105161136_sandboxes_name_lower_index.up.sqldid (CONCURRENTLY, the same key), with that file's recovery notes, adapted to a down.#1522's files are unchanged, so their checksums still match.
Validation
scripts/check-postgres-migrations-isolated.sh,scripts/check-migration-versions.shandscripts/check-postgres-migrations-squawk.sh(squawk 2.69.0, 0 issues) pass againstmain.space-db-migrator control-planebuilt at this head ran (transcript):upfrom empty:20261105161136builds the index, then this migration drops it;down: the index is rebuilt valid, with db: index sandboxes by ID suffix and by lowercased name #1522's definition;upagain: dropped;down, the index dropped by hand, thenup:IF EXISTSdrops nothing and records the migration.space-db-migrateJob applied this one: the index was gone andidx_sandboxes_id_suffixkept. api: search the admin sandbox list by ID, and name owners and pools #1523's API and ui: search sandboxes on the server, show owners' emails, and drop unused controls #1524's console then passed the loop's search steps.Severity and risk
Severity: low. It removes write overhead and index storage; nothing is incorrect without it.
Risk: low. On
mainnothing reads the index; its only reader was #1523's previous head, which never merged.Rollback: run this migration's down, which rebuilds the index concurrently.
Rollout
A deployment that has run #1522's migration has already built
idx_sandboxes_name_lower. A deployment that runs migrations before this lands will build it too. The drop handles either case:IF EXISTSmakes it a no-op where the index is gone, andCONCURRENTLYkeeps writes flowing while it drops.A deploy that carries both #1522 and this migration builds the index and then drops it. That costs one concurrent build, and is harmless.
The drop runs under the migrator session's timeouts and is not retried. It waits for every transaction holding a lock on
sandboxes. If it fails, it records nothing, possibly leaving the index INVALID, and the next migrator run drops it.No API on
mainreads the index, so this can merge before or after #1523 and #1524.AI assistance: Claude Code drafted the migration and this description, and ran the checks.
🤖 Generated with Claude Code