test(cli): cover migration up and repair (CLI-2269) - #6376
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baa11077ba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase/cli/supabase@a8a869c24184bcc14d530f5bcc9ba46fec102142Preview package for commit |
|
/ai-review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Superseded by a newer AI review
🤖 AI Review
Both independent reviews completed. The PR only adds two live tests. Six findings are confirmed: one documented live-test convention violation, two test reliability/coverage gaps, and three nits. The claimed redundant file-cleanup error handling is refuted because it handles non-ENOENT failures and allows subsequent cleanup to continue.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/legacy/commands/migration/repair/repair.live.test.ts:38 |
test-conventions |
claude | Both new live tests assert output from migration list even though trusted repository policy requires assertions to remain focused on the command under test. |
| 🟡 MINOR | apps/cli/src/legacy/commands/migration/up/up.live.test.ts:23 |
test-reliability |
claude | The migration up test depends on the shared project's remote migration history containing no version other than its single local migration, allowing leaked state from an earlier test to cause a cascading missing-local failure. |
| 🟡 MINOR | apps/cli/src/legacy/commands/migration/up/up.live.test.ts:30 |
test-coverage |
codex | The live test can pass without proving that migration up executed the migration's SQL statements. |
| ⚪ NIT | apps/cli/src/legacy/commands/migration/repair/repair.live.test.ts:18 |
clarity |
claude | The repair test writes table-creation SQL that is stored in migration history but never executed, without documenting that behavior. |
| ⚪ NIT | apps/cli/src/legacy/commands/migration/repair/repair.live.test.ts:58 |
test-quality |
claude | The repair test performs a redundant third migration repair --status reverted subprocess on its successful path. |
| ⚪ NIT | apps/cli/src/legacy/commands/migration/up/up.live.test.ts:12 |
duplication |
claude | The PR adds another inline copy of a migration-version generator that is already duplicated across live tests. |
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/src/legacy/commands/migration/up/up.live.test.ts:34(redundancy): The migration-file cleanup and its try/catch are redundant becauseforce: truesuppresses missing-file errors and the workspace fixture later deletes the entire temporary directory.
Refuted:force: truesuppresses only a missing-path error, not other filesystem failures. The catch records those failures and, importantly, permits the database cleanup at lines 39-64 to continue; relying on the later fixture cleanup does not provide that sequencing or aggregation.
Stats
Claude findings: 6 · Codex findings: 1 · Confirmed: 6 · Refuted: 1 · Uncertain: 0
Models: claude-opus-5 + gpt-5.6-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9499bdb509
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
2 participants
TL;DR
adds live e2e coverage for
migration upandmigration repair, closing the migration family...whats introduced?
migration up: applies a test written migration to the remote database, proven by the apply banner and the history row migration list reads backmigration repair: inserts a history row with status applied, proves it through migration list, then removes it with status reverted and proves the absenceref: