fix: repair invalid concurrently created indexes

[ferhatelmas]

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Concurrent index creation can fail, retry would skips due to exists check.

What is the new behavior?

For index migrations, extract targets and probe if index is valid or not.
If invalid, drop the index so that runner can recreate.
This makes the migration retryable.

Additional context

Related to #1353 and needed before.

[Copilot]

🟡 Changes recommended

A newly added test file uses vi without importing it, which can break builds/typechecking depending on Vitest global configuration.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR makes concurrent index migrations retryable by detecting and dropping invalid indexes left behind by failed CREATE INDEX CONCURRENTLY attempts, so subsequent retries don’t get skipped by IF NOT EXISTS.

Changes:

• Added a pre-migration guard that parses concurrent index targets and drops invalid leftover indexes before running the migration.
• Added unit/integration-style tests covering invalid/valid/conflicting index states and the OrioleDB “no CONCURRENTLY” path.
• Adjusted migration test scaffolding/mocks to support the new guard behavior.

File summaries

File	Description
src/internal/database/migrations/migrate.ts	Runs the new repair step before executing each transformed migration and logs when repairs occur.
src/internal/database/migrations/migrate.test.ts	Adds end-to-end migration-runner tests validating drop-before-retry behavior and OrioleDB behavior.
src/internal/database/migrations/concurrent-index-guard.ts	Implements SQL scanning + catalog probing to safely drop only invalid targeted concurrent indexes.
src/internal/database/migrations/concurrent-index-guard.test.ts	Adds focused tests for parsing/guard behavior across comments/strings/tenant migrations.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[coveralls]

Coverage Report for CI Build 33689446983

Coverage increased (+0.2%) to 81.997%

Details

• Coverage increased (+0.2%) from the base build.
• Patch coverage: 4 uncovered changes across 1 file (104 of 108 lines covered, 96.3%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
src/internal/database/migrations/concurrent-index-guard.ts	103	99	96.12%
Total (2 files)	108	104	96.3%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	13382
Covered Lines:	11418
Line Coverage:	85.32%
Relevant Branches:	7964
Covered Branches:	6085
Branch Coverage:	76.41%
Branches in Coverage %:	Yes
Coverage Strength:	634.23 hits per line

---

💛 - Coveralls

[claude]

I reviewed this PR and didn't find any bugs. Because it introduces a hand-rolled SQL scanner plus logic that runs DROP INDEX CONCURRENTLY against production schemas ahead of every pending migration, a human look would still be worthwhile before merging.

What was reviewed: the SQL literal/comment/dollar-quote scanner and CONCURRENT_INDEX_CREATE regex used to extract CREATE INDEX CONCURRENTLY targets; the pg_catalog validity/ownership query and the drop-and-log repair path; and the migrate.ts integration point plus the new test coverage in both test files. Also checked (and ruled out) whether partitioned-table parent/child index states could be misclassified, and whether running the repair before every migration (not just retries) could spuriously throw via concurrentIndexTargets's target-count assertion.

Extended reasoning...

Overview

The PR adds concurrent-index-guard.ts, a new module that parses migration SQL with a custom regex/state-machine scanner to find CREATE INDEX CONCURRENTLY targets, checks each target's validity via pg_catalog, and drops any invalid index so a retried migration rebuilds it instead of silently skipping via IF NOT EXISTS. migrate.ts wires this in immediately before each pending migration runs, logging a warning when indexes are removed. Both new and existing test files gain corresponding coverage.

Security risks

The identifiers used to build the DROP INDEX CONCURRENTLY statement come from parsing the migration's own SQL and from pg_catalog rows keyed off the parsed table/index reference, not from external/user input, and are re-quoted via escapeAndQuoteIdentifier before use — this isn't exposed to request-time attacker input. No injection, auth, or data-exposure concerns were identified.

Level of scrutiny

This warrants a closer-than-default look: it is a hand-rolled SQL parser (quote/comment/dollar-quote handling, escape sequences) feeding a query that can execute DROP INDEX CONCURRENTLY in the migration path used across all tenant databases. A parsing edge case or an incorrect validity judgement could either fail to repair a broken index or, worse, misidentify and drop an unrelated valid index. The logic is intricate enough (244 new lines, several helper functions for lexing SQL) that even with passing tests, correctness across the full space of valid Postgres CREATE INDEX CONCURRENTLY syntax variations (schema-qualified names, USING clauses, expressions, partitioned tables) is hard to fully verify from a single automated pass.

Other factors

No bugs were surfaced by the automated hunt, and two candidate issues (partitioned-table parent/child index validity, and the repair running on every pending migration rather than only on retries) were investigated and considered not blocking. Test coverage is present for both the standalone guard module and its integration into migrate.ts. Given the change is self-contained to migration tooling (not a hot request path) but still touches a critical, hard-to-fully-exercise-in-CI code path (real Postgres pg_catalog state and concurrent index semantics), a human review is a reasonable additional step before merge.

[Copilot]

🟡 Changes recommended

A new test contains a hardcoded assertion that is likely to become brittle as tenant migrations evolve, causing avoidable CI failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are scoped, add targeted test coverage for the new behavior and failure modes, and integrate safely into the existing migration execution flow.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite