Skip to content

refactor(cli): cover db schema declarative sync with effect lint (CLI-2409) - #6820

Merged
7ttp merged 4 commits into
7ttp/cli-2409-db-family-coverage-schema-generatefrom
7ttp/cli-2409-db-family-coverage-schema-sync
Sep 25, 2026
Merged

7ttp merged 4 commits into
7ttp/cli-2409-db-family-coverage-schema-generatefrom
7ttp/cli-2409-db-family-coverage-schema-sync

Conversation

@7ttp

@7ttp 7ttp commented Sep 24, 2026

Copy link
Copy Markdown
Member

TL;DR

brings supabase db schema declarative sync under the effect lint, which completes the db family

whats introduced?

effect lint applied to db schema declarative sync:

  • the per subcommand allow list entries collapse into one db/** entry
  • typed errors are yielded directly
  • sync timestamps come from DateTime with the same output
  • tests use FileSystem and Path, set env through withEnvVar, and pin the transaction mode message
  • the e2e test runs through the effect native harness with its project in acquireUseRelease

ref:

@7ttp 7ttp self-assigned this Sep 24, 2026
@7ttp
7ttp added this pull request to stack #6813 September 24, 2026 20:57
@7ttp
7ttp requested a review from a team as a code owner September 24, 2026 20:57

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤖 AI Review

One minor test coverage gap is confirmed. The lint coverage finding is refuted: the new negated glob includes the db tree in Effect lint. No additional defect was verified. Tests were not run because dependencies are absent.

Findings

Severity Location Category Sources Claim
🟡 MINOR apps/cli/src/commands/db/schema/declarative/sync/sync.integration.test.ts:1532 test-coverage claude The new invalid transaction mode test checks only the error message. It does not verify the outward error tag or that no migration file was written.
Refuted findings (kept for transparency, not posted as review comments)
  • .oxlintrc.effect.json:15 (lint-coverage): The new glob excludes the entire db command tree from Effect lint, including the changed sync code.
    Refuted: The ! pattern reverses the initial ignore pattern and includes the db tree in lint coverage. The trusted baseline config uses the same negated pattern convention for other included command directories.

Stats

Claude findings: 1 · Codex findings: 1 · Confirmed: 1 · Refuted: 1 · Uncertain: 0


Models: claude-opus-5-5 + gpt-6-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

@avallete avallete left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Effect-lint coverage for declarative sync looks mechanical.

Prompt and compatibility failures are the same tagged errors, now yielded directly. The oxlint include widens to commands/db/** now that the stack covers that tree. Tests are still there; the unknown pg-delta transaction mode case was added.

@avallete
avallete added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue because a pull request earlier in the stack was removed Sep 25, 2026
@7ttp
7ttp added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue because a pull request earlier in the stack was removed Sep 25, 2026
@7ttp
7ttp added this pull request to the merge queue Sep 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue because a pull request earlier in the stack was removed Sep 25, 2026
@7ttp
7ttp added this pull request to the merge queue Sep 25, 2026
Merged via the queue into develop with commit 40e7fd1 Sep 25, 2026
27 of 41 checks passed
@7ttp
7ttp deleted the 7ttp/cli-2409-db-family-coverage-schema-sync branch September 25, 2026 10:44
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