Repository navigation
chore(repo): sync develop into next - #7196
Conversation
## TL;DR most of `/shared` already followed the effect skill after its effect lint coverage in CLI-2458, this picks up the few things that were left ## whats introduced? - functions deploy retries and rate limit waits move from hand written loops to `Effect.retry` and `Effect.repeat` with a `Schedule`, keeping the same attempts, delays and messages - `serve` outcomes become `Data.TaggedEnum` values matched with `$match`, and the shutdown grace uses `Effect.timeoutOrElse` - `Predicate.isTagged` and exhaustive matching replace `_tag` checks, so the trace sanitizer now handles every decision explicitly - casts and non null assertions give way to narrowing, and `consent` encodes through Effect - the feedback client and project link state use `Effect.fn` - tests run on `it.effect` wherever the clock does not matter, and the malformed ref services test now asserts no request is sent ## notes - no user facing behaviour changes at all - the four new spans only appear in trace output and the `SUPABASE_DEBUG=1` span console ## ref - extends: #7122
# Conflicts: # apps/cli/src/shared/config/cli-project-context.layer.unit.test.ts # apps/cli/src/shared/init/project-init.modes.integration.test.ts
Claude resolved the merge of
|
| File | Resolution |
|---|---|
apps/cli/src/shared/config/cli-project-context.layer.unit.test.ts |
Kept next's new test "exposes project .env values except names the shell sets" and the existing empty-context test. Switched both to develop's it.effect instead of it.live. |
apps/cli/src/shared/init/project-init.modes.integration.test.ts |
Used next's wording, which drops the "Go parity" phrasing from the describe and test names. Kept develop's change from it.live to it.effect. |
Quick checks. The merged tree passes check:all and unit tests.
Approve to fast-forward next once checks pass (workflow run).
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 AI Review
Both independent reviews were available. Confirmed Claude's single test-coverage nit and corrected its line reference. Codex reported no findings. This establishes a missing assertion, not a production retry bug. Tests were not run.
Findings
| Severity | Location | Category | Sources | Claim |
|---|---|---|---|---|
| ⚪ NIT | apps/cli/src/commands/functions/deploy/deploy.integration.test.ts:1204 |
test-coverage |
claude | The persistent-rate-limit test consumes eight retry notices but never checks that no additional notice remains after deployment fails. A misleading final retry notice could therefore pass this test. |
Stats
Claude findings: 1 · Codex findings: 0 · Confirmed: 1 · Refuted: 0 · 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.
| const error = yield* Fiber.join(deploy).pipe(Effect.flip); | ||
|
|
||
| expect(error).toBeInstanceOf(FunctionsApiStatusError); | ||
| expect(error).toMatchObject({ status: 429 }); | ||
| expect(deployTimes).toEqual([ | ||
| 0, 1_000, 3_000, 7_000, 15_000, 31_000, 63_000, 95_000, 127_000, | ||
| ]); |
There was a problem hiding this comment.
⚪ NIT · test-coverage · source: claude
The persistent-rate-limit test consumes eight retry notices but never checks that no additional notice remains after deployment fails. A misleading final retry notice could therefore pass this test.
Evidence: The harness queues every rate-limit notice at lines 735-742. The test creates an unbounded queue at line 1179, consumes eight notices at lines 1198-1203, then checks only the error and request timestamps at lines 1204-1210.
Suggested fix: After joining the deployment fiber, assert that the notices queue is empty.
There was a problem hiding this comment.
This persistent-rate-limit test came from develop and the merge did not change it, so the missing empty-queue check was not introduced by the merge. It is out of scope for the sync and can be handled in a follow-up on develop.
Claude repair roundNo CI job failed on this sync, so I made no code changes. The one review finding is about a test 0 findings fixed, 1 declined (workflow run). |
supabase-oss
left a comment
supabase-oss
left a comment
There was a problem hiding this comment.
CI is green and the AI review raised nothing the merge introduced, so this approves Claude's resolution of the sync (https://github.com/supabase/cli/actions/runs/38881418199).
2 participants
Merging
developintonextconflicts. This branch starts at the tip ofdevelop(f592d85);nextis atb4a24ea.Conflicting files:
apps/cli/src/shared/config/cli-project-context.layer.unit.test.tsapps/cli/src/shared/init/project-init.modes.integration.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.