Repository navigation
chore(repo): sync develop into next - #7131
Conversation
`Run integration tests` ran every package's integration suite in one job, with the stack suite as the long pole. This turns it into a matrix: the stack suite in three Vitest shards, the CLI suite in two, and the API suite plus the stack namespace e2e tests in their own entry. The `Run unit and integration tests` summary gate still aggregates the matrix result. CLI e2e files run serially, so a shard can be no shorter than its longest file, and `start.lifecycle.e2e.test.ts` alone set that floor. Its two health-check scenarios move to `start.health.e2e.test.ts`, the e2e timings are refreshed from a current run, and CLI e2e runs in five shards instead of four. ## Effect on the Test workflow Compared with the same workflow on `develop`, excluding time spent waiting for a runner: - The slowest integration job is about 55% shorter, so integration tests are no longer on the critical path. - The slowest CLI e2e shard spends about 15% less time in tests. One shard still runs about 25% over the others, so the timing-based split is not yet even. - The workflow as a whole finishes about 20% sooner. It is now bounded by the CLI and stack e2e jobs, which end within a few seconds of each other. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… (#7117) ## TL;DR brings the `migration` module under the effect lint ## whats introduced? - the `migration*` and `migrate-and-seed` files join the allow list, with the sql helpers they run through - (`postgres-url`, `sql-split`, `param-introspection`, `pg-net-guidance`, `drop-objects`, `pg-dump` and `bundled-postgres-client`), and `postgres-client.run` widens to its tests - `checkScannerBufferSize` reads `SUPABASE_SCANNER_BUFFER_SIZE` through `Config`, still ahead of the project env - migration timestamps go through `DateTime`, and `migration fetch` yields its errors directly with the same messages - the migration and seed tests swap `node:fs` and `process.env` for scoped temp dirs and `withConfigEnv`, and the `pg-dump` and some `migration-history` tests move to `it.effect` ## ref - related: CLI-2654
CLI-2654) (#7118) ## TL;DR brings the `seed` and `pgdelta` modules under the effect lint ## whats introduced? - the `seed*`, `pgdelta*` and `diff-engine` files join the allow list - `isPgDeltaDebugEnabled` reads `PGDELTA_DEBUG` through `Config`, so `db diff`, `db pull` and the declarative `generate` and `sync` yield it - `seed-ops` fails the scanner check with `MigrationSeedError` instead of a bare `Error`, with the same message - `seed-buckets` yields its errors directly and recovers with `orElseSucceed` - the pgdelta tests pin `PGDELTA_DEBUG` through `withConfigEnv`, and the ssl probe test owns its server through `acquireRelease` ## ref - closes: CLI-2654
# Conflicts: # apps/cli/src/command-internal/db-pull-run.ts # apps/cli/src/command-internal/migrate-and-seed.unit.test.ts # apps/cli/src/command-internal/migration-apply.ts # apps/cli/src/command-internal/migration-apply.unit.test.ts # apps/cli/src/command-internal/migration-history.unit.test.ts # apps/cli/src/command-internal/pgdelta.ts # apps/cli/src/command-internal/pgdelta.unit.test.ts # apps/cli/src/command-internal/seed-ops.unit.test.ts # apps/cli/src/command-internal/seed.unit.test.ts # apps/cli/src/commands/db/diff/diff.integration.test.ts # apps/cli/src/commands/start/start.lifecycle.e2e.test.ts
Claude resolved the merge of
|
| File | Resolution |
|---|---|
apps/cli/src/command-internal/pgdelta.ts |
Took next's isPgDeltaDebugEnabled built on envValue (an Effect<boolean>, same call shape as develop's Config version) and dropped the unused Config/Option/Path imports. |
apps/cli/src/command-internal/db-pull-run.ts |
Used next's pgDeltaDebug (read once near the top) for the shadow diff. Removed the duplicate shadowing const pgDeltaDebug that the declarative-export branch had redeclared. |
apps/cli/src/command-internal/migration-apply.ts |
Took next's checkScannerBufferSize body: reads through envValue, uses the constants without the GO_ prefix, and says scanner: token too long. Removed the now-unused Config/Option imports. |
apps/cli/src/command-internal/pgdelta.unit.test.ts |
Merged next's truthy-value test with develop's withEmptyConfigEnv and unresolvable-provider tests. All of them pass against the envValue implementation, which falls back to false. |
apps/cli/src/command-internal/migration-history.unit.test.ts |
Kept develop's it.effect/Effect.exit conversions and used next's describe titles without Go references. |
apps/cli/src/command-internal/seed.unit.test.ts |
Kept develop's Effect-native tests with next's titles and the updated scanner: token too long message. Dropped next's cliConfigProviderLayer wiring because withConfigEnv already pins the env. |
apps/cli/src/command-internal/seed-ops.unit.test.ts |
Kept develop's layer(BunServices.layer) and withConfigEnv tests with next's titles and the updated scanner: error text. |
apps/cli/src/command-internal/migrate-and-seed.unit.test.ts |
Kept develop's Effect-native tests and ported next's changes into them: the new schema_paths-ignored warning assertions, the renamed pg-delta fallback test, and the new 'migrations disabled + pg-delta enabled' test. Also used next's suggestion test title. |
apps/cli/src/command-internal/migration-apply.unit.test.ts |
Kept develop's schemaFile/applySchema/withConfigEnv test style throughout, with next's Go-free titles and scanner: token too long assertions. Removed the unused cliConfigProviderLayer/Layer imports; develop's read-phase config-error test was rewritten to match next's swallow-to-unset behavior. |
apps/cli/src/commands/db/diff/diff.integration.test.ts |
Kept both sides: develop's PGDELTA_DEBUG parameterized test, followed by next's tests for the pg-delta default, the migra config toggle and the declarative advisory. |
apps/cli/src/commands/start/start.lifecycle.e2e.test.ts |
Used next's import layout (sanitizeProjectId from shared/config/project-id.ts). Dropped the getRegistryImageUrl import, which develop had already stopped using. |
apps/cli/src/commands/start/start.health.e2e.test.ts |
Semantic fix: next moved sanitizeProjectId out of docker-ids.ts, so this file now imports it from shared/config/project-id.ts. |
Decisions for @supabase/cli. Confirm them, or push a change to this branch:
- If the config provider can't resolve SUPABASE_SCANNER_BUFFER_SIZE, should that fail the read (develop) or count as unset (next's
envValue)? Chose: Next's behavior: an unresolvable value counts as unset and the project env is used instead. Develop's 'fails with a read-phase error' test was rewritten to check that fallback, so the provider-failure path is still covered. - Which scanner error text should the merged tests expect:
bufio.Scanner: token too long(develop's tests) orscanner: token too long(next)? Chose: Next'sscanner: token too long, since next deliberately removed Go wording from the implementation. Every develop test assertion was updated to match.
Quick checks. The merged tree passes check:all and unit tests.
Approve to fast-forward next once checks pass (workflow run).
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews completed: Claude reported three findings and Codex reported none. One minor timestamp-formatting concern is confirmed; the seed-error and CI-check findings are refuted. The timestamp behavior predates this PR, but the new test preserves the defect. No test suite was run.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| 🟡 MINOR | apps/cli/src/command-internal/migration-timestamp.format.unit.test.ts:33 |
timestamp-formatting |
claude | The new test preserves an existing formatting defect: valid timestamps with years 0000–0099 pass through unchanged because Date.UTC maps those years to 1900–1999. |
Refuted findings (kept for transparency, not posted as review comments)
apps/cli/src/command-internal/seed-ops.ts:121(code-quality): seedData introduces an unnecessary dependency on MigrationSeedError because its outer mapError discards that intermediate error type.
Refuted: The trusted root CLAUDE.md makes the Effect conventions authoritative, and its referenced guide prescribes tagged errors for expected failures. This change replaces a plain Error with the existing seed-domain tagged error, also used for scanner failures in seed.ts:137. Remapping an internal failure at the caller boundary does not invalidate that convention. The proposed direct use of mapError would additionally invoke the caller mapper twice through the existing outer mapError; its unconstrained generic E need not contain a message field..github/workflows/test.yml:111(ci): Renaming integration and E2E matrix checks can strand required checks because the workflow lacks a stable integration aggregator.
Refuted: The claimed missing aggregator exists: test.yml:239-261 retains "Run unit and integration tests", depends on test-integration, and checks its aggregate result. Lines 407-427 similarly retain "Run end-to-end tests" and depend on the E2E matrix. pr/.github/scripts/fast-forward.ts:15-21 lists those stable summary names as required checks, matching trusted/.github/MAINTAINERS.md:199-203 for develop and next. The documented required checks remain unchanged.
Stats
Claude findings: 3 · Codex findings: 0 · Confirmed: 1 · Refuted: 2 · Uncertain: 0
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: manual · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
| it("passes through years below 100, unlike Go's time.Parse", () => { | ||
| expect(formatTimestampVersion("00991231000000")).toBe("00991231000000"); | ||
| expect(formatTimestampVersion("01000101000000")).toBe("0100-01-01 00:00:00"); | ||
| }); |
There was a problem hiding this comment.
🟡 MINOR · timestamp-formatting · source: claude
The new test preserves an existing formatting defect: valid timestamps with years 0000–0099 pass through unchanged because Date.UTC maps those years to 1900–1999.
Evidence: migration-timestamp.format.unit.test.ts:33-36 expects "00991231000000" unchanged. migration-timestamp.format.ts:29-33 constructs the date with Date.UTC and rejects the resulting year mismatch, despite lines 3-6 documenting passthrough for invalid timestamps.
Suggested fix: Validate calendar dates without Date.UTC's special handling of years 0–99, and change the test to expect "0099-12-31 00:00:00".
There was a problem hiding this comment.
The Date.UTC handling of years 0–99 in formatTimestampVersion, and the test that expects it, were already on develop and not introduced by this merge. It is out of scope for the sync and should go in a separate follow-up.
Claude repair roundNo CI check failed on this sync, so no code changes were needed. The only review finding concerns timestamp formatting that 0 findings fixed, 1 declined (workflow run). |
Merging
developintonextconflicts. This branch starts at the tip ofdevelop(02f0908);nextis at66f816a.Conflicting files:
apps/cli/src/command-internal/db-pull-run.tsapps/cli/src/command-internal/migrate-and-seed.unit.test.tsapps/cli/src/command-internal/migration-apply.tsapps/cli/src/command-internal/migration-apply.unit.test.tsapps/cli/src/command-internal/migration-history.unit.test.tsapps/cli/src/command-internal/pgdelta.tsapps/cli/src/command-internal/pgdelta.unit.test.tsapps/cli/src/command-internal/seed-ops.unit.test.tsapps/cli/src/command-internal/seed.unit.test.tsapps/cli/src/commands/db/diff/diff.integration.test.tsapps/cli/src/commands/start/start.lifecycle.e2e.test.tsTo resolve:
git fetch origin git switch sync/develop-into-next git merge origin/next # resolve the conflicts, then commit git push origin sync/develop-into-nextApprove to fast-forward
next; do not use the merge button.