feat(deps): upgrade alchemy to beta.67 and effect to beta.103 - #203
Conversation
|
Warning Review limit reached
Next review available in: 12 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (29)
Summary by CodeRabbit
WalkthroughThe change upgrades Alchemy, Effect, and related Effect platform packages across examples and packages. It applies an Alchemy declaration patch for optional 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-npm-effect-resolution.mjs`:
- Around line 15-18: Update the adversarial-tree comment above the fixture to
describe the current mismatch created by the effect override in
checkAdversarialShape, rather than claiming the scenario involves an app
dependency on `@effect/platform-node-shared` or peer hoisting. Keep the
description consistent with the actual fixture behavior.
In `@skills-contrib/upgrade-alchemy-effect/SKILL.md`:
- Line 75: Add blank lines before and after every fenced code example in
SKILL.md, including the fences around the examples at the referenced locations,
so markdownlint MD031 passes without changing the example content.
- Around line 88-91: Update the package.json patchedDependencies entry before
the Step 3 pnpm install: remove the stale alchemy@2.0.0-beta.67
patch/configuration, or retarget it to the bumped package and exact version if
the patch remains applicable. Ensure the patch references the correct package
after the dependency move and no obsolete key remains.
- Around line 39-50: Update the dependency description in the introductory
paragraph to call each companion’s peer requirement a “matching beta range”
rather than an “own exact version,” while preserving the existing guidance that
all Effect packages must use the same beta track.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9bed849f-9f63-43bb-b7af-f52062a366af
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
examples/bucket/package.jsonexamples/cron/package.jsonexamples/pn-widgets/package.jsonexamples/storage/package.jsonexamples/store/package.jsonexamples/storefront-auth/package.jsonpackage.jsonpackages/0-framework/1-core/core/package.jsonpackages/1-prisma-cloud/0-lowering/local-target/package.jsonpackages/1-prisma-cloud/0-lowering/local-target/src/__tests__/postgres-instance-name-drift.test.tspackages/1-prisma-cloud/0-lowering/lowering/package.jsonpackages/1-prisma-cloud/0-lowering/lowering/src/__tests__/ComputeService.test.tspackages/1-prisma-cloud/0-lowering/lowering/src/__tests__/ServiceKey.test.tspackages/1-prisma-cloud/0-lowering/lowering/src/compute/ComputeService.tspackages/1-prisma-cloud/0-lowering/lowering/src/compute/Deployment.tspackages/1-prisma-cloud/0-lowering/lowering/src/state/layer.tspackages/1-prisma-cloud/0-lowering/lowering/src/state/transient.tspackages/1-prisma-cloud/1-extensions/target/package.jsonpackages/1-prisma-cloud/1-extensions/target/src/__tests__/generated-param.test.tspackages/1-prisma-cloud/1-extensions/target/src/__tests__/pg-warm-resource.test.tspackages/1-prisma-cloud/1-extensions/target/src/__tests__/pn-migration-resource.test.tspackages/1-prisma-cloud/1-extensions/target/src/__tests__/s3-credentials.test.tspackages/9-public/composer-prisma-cloud/package.jsonpackages/9-public/composer/package.jsonpatches/alchemy@2.0.0-beta.67.patchscripts/check-npm-effect-resolution.mjsskills-contrib/upgrade-alchemy-effect/SKILL.mdtest/integration/package.jsonwebsite/package.json
The three examples that declare alchemy directly also declare @effect/platform-bun, so pnpm peer-resolves their alchemy instance against that version — at 4.0.0-beta.97 it wanted effect@^4.0.0-beta.97 while the workspace pins 4.0.0-beta.93, which is why the lockfile carried an alchemy entry resolved against a second effect. pnpm only warns, so nothing broke; it just contradicted the constellation the public packages pin. Aligning rather than deleting the declaration: these examples depend on alchemy directly, so removing it would leave that instance without the optional peer its CLI loads under bun (the failure c26200b fixed). Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Drops a stale @effect/vitest@4.0.0-beta.92 pair and a duplicated @prisma/management-api-sdk entry. No package.json changes and no version moves outside the effect constellation this aligns to. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
The old pins were the whole reason TML-3158 existed. alchemy 2.0.0-beta.59
called Schedule.either/Schedule.both, which effect removed at beta.97, so we
were held at effect 4.0.0-beta.93 — and every @effect/* companion had to be
held there with it, since each peers its own exact version. Any dependency
that floated past that line dragged an incompatible effect into the tree.
alchemy dropped those calls and now peers effect >=4.0.0-beta.100, so the
constraint is gone. This moves the whole constellation to 4.0.0-beta.103,
where every companion peers ^4.0.0-beta.103 and the set is self-consistent.
Two code changes the upgrade forces:
Schedule.both(spaced(x), during(y)) — intersection, "retry every x while
within y" — no longer exists. Schedule.upTo({ duration: y }) says the same
thing more directly: it bounds an existing schedule by elapsed time while
preserving its delay behavior.
Resource handler contexts gained a required fqn, so the test fixtures that
build those contexts by hand now supply it.
The patch is a types-only upstream fix. alchemy declares Aliases as
`Aliases?: readonly string[]` on ResourceClassLike but
`Aliases: readonly string[] | undefined` on ResourceClass; under this repo
exactOptionalPropertyTypes those do not match, so every Provider.effect and
Provider.collection call failed to compile. alchemy carries @ts-expect-error
at its own equivalent call sites, so the inconsistency is upstream. Adding
`| undefined` to the optional property fixes all 45 of them at the source,
with no casts and no call-site churn.
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
…hoisting Two assertions in the resolution check encoded the old alchemy: that the resolved effect still has Schedule.either, and an adversarial fixture that depended on a published release whose effect peer sits above our pin. Neither survives the upgrade. Schedule.either is legitimately gone at beta.103, and with the pin now at the newest published beta there is no newer release to build the adversarial tree from — so the shape it created stopped being adversarial at all, which the check caught and reported. The presence probe was only ever standing in for "alchemy can run on this effect"; assertCliStarts answers that directly, since starting the built bin loads alchemy provider tree. And the adversarial shape now pins a different effect with an npm override rather than waiting for the registry to supply one, so it keeps proving the thing that matters — when alchemy resolves an effect we did not pin, the CLI says so — without being hostage to where our pin happens to sit. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
…ellation Writes down what this upgrade cost to learn. The headline is the cheap check nobody ran: TML-3158 was worked around through pinning, a peer-dependency experiment and a CLI preflight, while a newer alchemy that had already dropped the removed effect APIs sat on the registry the whole time. The rest is the material that is not obvious from the code: why effect and every @effect/* companion have to move as one set, why pnpm hides a broken constellation that npm exposes to consumers, the breakage classes an upgrade produces, when the alchemy type patch can be dropped, and the two ways the resolution check silently stops proving anything. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Review follow-ups. `^4.0.0-beta.103` is a floor, not an exact pin — it accepts beta.104 and stable 4.x, and rejects only versions below it, so the rule is that effect must not be older than any companion in the tree. The patch step also came too late: pnpm.patchedDependencies is keyed by exact version, so bumping alchemy before clearing it leaves the key pointing at a version that is no longer installed. Clearing it now precedes the install. The resolution check header still described the hoisting fixture it no longer uses. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
99d1ae6 to
bca19a7
Compare
Until this PR, every package in the repo was held here:
That pin is the reason TML-3158 existed at all. alchemy beta.59 called
Schedule.eitherandSchedule.both, whicheffectremoved at beta.97 — so we were held at beta.93, every@effect/*companion had to be held there too (each peers its own exact version), and any dependency that floated past that line dragged an incompatibleeffectinto a consumer tree. #196 pinned the constellation and #202 made the CLI refuse to run on a bad tree, but both work around the old alchemy.The decision
Upgrade out of it. alchemy beta.67 no longer calls the removed combinators and peers
effect >=4.0.0-beta.100, so the constraint is gone:This removes the cause rather than another symptom. The start-up check from #202 stays — alchemy's peer range is still open-ended, so a future
effectcan break the same way, and the check is what turns that into a sentence instead of aTypeError.What the upgrade forces
Eight betas of a pre-1.0 tool moved real API. Three things had to change, and one had to be fixed upstream:
Schedule.bothis gone. We usedSchedule.both(spaced(x), during(y))— the intersection, meaning "retry every x, but only while within y" — in four places.Schedule.upTo({ duration: y })states it more directly: it bounds an existing schedule by elapsed time while preserving its delay behavior.fqn, so the six test fixtures that build those contexts by hand now supply it.patches/alchemy@2.0.0-beta.67.patch). alchemy declaresAliases?: readonly string[]onResourceClassLikebutAliases: readonly string[] | undefinedonResourceClass. Under this repo'sexactOptionalPropertyTypes, an explicitundefinedis not assignable to an exact-optional property, so everyProvider.effectandProvider.collectioncall stopped compiling — 45 errors across three packages. alchemy carries@ts-expect-errorat its own equivalent call sites, so this is upstream's inconsistency, not our misuse. Adding| undefinedto the optional property fixes all of them at the source. Worth reporting upstream.The first two commits are the smaller change this PR started as: aligning the three examples that still pinned
@effect/platform-bun@4.0.0-beta.97, plus apnpm dedupe. They are now subsumed by the upgrade but kept separate so the lockfile churn stays attributable.Why the resolution check changed
Two of its assertions encoded the old alchemy and could not survive:
effectstill hadSchedule.either. That was only ever a stand-in for "alchemy can run on thiseffect", and at beta.103 it is legitimately absent.assertCliStartsanswers the same question directly, because starting the built bin loads alchemy's provider tree — the thing that used to crash.effectpeer sat above our pin. With the pin now at the newest beta, no such release exists, so the shape stopped being adversarial — which the check itself detected and reported, as designed. It now pins a differenteffectwith an npmoverride, building the same end state deterministically instead of waiting for the registry to supply one.Verification
typecheck(74 tasks),build,lint,lint:deps,install --frozen-lockfile, and all three shapes ofcheck:npm-effect-resolutionpass. The full test suite is at parity withmain: 60/62, with the same two packages (local-target,dev-emulators) failing only under the fully parallel run and passing in isolation on both branches. The E2E deploy jobs are the real check on an alchemy upgrade, and they run on this PR.Alternatives considered
effectfor the whole repo on a transitive constraint most of the code has no stake in.effectat the minimum it accepts (beta.100). No benefit: the companions all publish matching beta.103, and picking a lower point in the range means another bump later for nothing.Aliasestype mismatch at the call sites with the repo'sblindCasthelper, or by wrapping each resource. Tried first: it needs a cast at ~20 sites, and narrowing the argument type also breaks inference for the second argument, which surfaced a fresh wave of errors. Fixing the declaration is one line, needs no casts, and preserves inference.🤖 Generated with Claude Code