build(server-release): prune foreign-platform and unreachable Prisma artifacts - #231
build(server-release): prune foreign-platform and unreachable Prisma artifacts#231Miista wants to merge 3 commits into
Conversation
WalkthroughThe server artifact build now prunes Prisma files that do not match the target platform. It removes unreachable provider runtimes and sourcemaps before validating required engines. Tests cover supported targets, cleanup behavior, retained files, missing directories, and filesystem errors. ChangesPrisma artifact pruning
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ServerArtifactBuild
participant PrismaPruning
participant PayloadDirectory
participant EngineValidation
ServerArtifactBuild->>PrismaPruning: Prune for target
PrismaPruning->>PayloadDirectory: Inspect and remove incompatible files
ServerArtifactBuild->>EngineValidation: Validate remaining engines
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.ts`:
- Line 42: Update both readdir handlers in buildServerBinaryArtifactPayload to
return an empty list only when the filesystem error code is ENOENT, and rethrow
all other errors. Add a test covering an expected directory path that is
actually a file, asserting that pruning rejects instead of silently continuing.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: adb50fb1-1174-432e-89c3-754c14d0fef7
📒 Files selected for processing (2)
packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.prismaEnginePrune.test.tspackages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.ts
Greptile SummaryThe PR reduces packaged server artifacts by pruning non-target Prisma native engines, unreachable provider WASM bundles, and runtime sourcemaps before validating the retained target engine.
Confidence Score: 4/5The PR appears safe to merge, with one non-blocking concern that unexpected filesystem errors can silently leave release artifacts unpruned. The retained engine names align with current library-mode runtime consumers and reachable database providers, while blanket readdir error suppression can conceal incomplete pruning without breaking engine-presence validation. Files Needing Attention: packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.ts
|
| Filename | Overview |
|---|---|
| packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.ts | Adds coherent target-aware Prisma pruning, but unexpected directory-read failures are silently treated as successful no-ops. |
| packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.prismaEnginePrune.test.ts | Covers target-specific retention, runtime pruning, and absent-directory behavior with focused filesystem fixtures. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Stage server sidecars] --> B[Prune non-target native engines]
B --> C[Prune unreachable WASM and sourcemaps]
C --> D[Validate target native engines]
D --> E[Finalize release payload]
Reviews (1): Last reviewed commit: "build(server-release): prune foreign-pla..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // ever runs on the one platform it was built for. Keep only that target's engine file. | ||
| async function pruneNonTargetPrismaEngineFiles(directoryPath: string, target: BinaryTarget): Promise<void> { | ||
| const keepFileName = resolvePrismaEngineFileNameForTarget(target); | ||
| const entries = await readdir(directoryPath, { withFileTypes: true }).catch(() => []); |
There was a problem hiding this comment.
If an unexpected filesystem error occurs while enumerating a staged Prisma directory, the blanket catch treats it as empty and silently skips pruning, allowing foreign engines, unreachable WASM files, or sourcemaps to remain in the release artifact while target-engine validation still passes.
Knowledge Base Used: packages/cli-common
…artifacts The packaged server binary artifact shipped Prisma query-engine binaries for all 5 platforms (schema.prisma's binaryTargets lists linux-x64, linux-arm64, darwin-x64, darwin-arm64, windows) even though a release build only ever targets one, plus WASM engines for cockroachdb/sqlserver (never a reachable ServerDbProvider) and .map sourcemaps in @prisma/client/runtime. On a linux-arm64 build this drops the artifact 656MB -> 392MB (-40%), verified against a real local build (real bun compile + Prisma generate) and confirmed both sqlite and mysql providers still resolve their query engine correctly at runtime in a container.
36fb948 to
1b51548
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Address CodeRabbit/Greptile review feedback on happier-dev#231: the pruning helpers swallowed every readdir failure into an empty directory, not just a missing one. A permission error or an expected directory path that's actually a file would silently skip pruning for that directory while validateServerPrismaEnginesForTarget still passes (it only checks that the kept engine file exists, not that pruning ran) -- letting foreign-platform engines, unreachable WASM engines, or sourcemaps survive into the release artifact undetected. Only ENOENT is now treated as "nothing to prune"; every other error propagates and fails the build.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.prismaEnginePrune.test.ts`:
- Line 146: Update the rejection assertion in the test around
readdirOrEmptyIfMissing to use toMatchObject({ code: 'ENOTDIR' }) instead of
only toThrow(). Preserve the existing scenario and verify that the propagated
filesystem error specifically has the ENOTDIR code.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0001ed1d-5858-4596-b798-10f97f4b98d4
📒 Files selected for processing (2)
packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.prismaEnginePrune.test.tspackages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli-common/src/componentArtifacts/buildServerBinaryArtifactPayload.ts
…test Address CodeRabbit follow-up on happier-dev#231: the new rejection test only asserted .rejects.toThrow(), which would also pass on an unrelated error. Assert the specific ENOTDIR code so the test can't silently pass for the wrong reason.
1 participant
Summary
The packaged server binary release artifact (used by the relay-server Docker image and
apps/stack's self-host installers) shipped Prisma query-engine binaries for all 5 platforms even though a single release build only ever runs on one:schema.prisma'sbinaryTargetslistslinux-x64,linux-arm64,darwin-x64,darwin-arm64,windows— a build for e.g.linux-arm64still had all 5 native.node/.dllfiles copied intogenerated/{sqlite,mysql}-clientandnode_modules/.prisma/client.node_modules/@prisma/client/runtimebundles WASM query engines for every database Prisma supports (postgresql, mysql, sqlite, cockroachdb, sqlserver) plus.mapsourcemaps for every bundled format —cockroachdb/sqlserverare never reachable (ServerDbProvideris only ever'sqlite' | 'mysql', withpostgresalways generated as the default), and sourcemaps are never needed by a production binary.The codebase already knows exactly which single engine file a build target needs (
resolvePrismaEngineFileNameForTarget), but that knowledge was only used for validating the artifact post-copy, never for pruning the other 4 platform variants.Fix
Added
pruneServerPrismaArtifactsForTarget()inbuildServerBinaryArtifactPayload.ts, called after sidecar staging and before the existing validation step (so validation now also proves pruning didn't break the kept engine):.node/.dllengine files fromgenerated/*-clientandnode_modules/.prisma/client.cockroachdb/sqlserverWASM engines and all.mapfiles fromnode_modules/@prisma/client/runtime.Impact
Compared against the currently-published
happierdev/relay-server:devimage (real OCI manifest layer sizes viadocker manifest inspect, arm64, digestsha256:ece74a90..., already includes the earlieree63f5d45Docker-side trim) versus the same Dockerfile recipe built locally with this fix applied (realbun compile+prisma generate, no mocking, pushed to a local registry to measure genuine compressed transfer size):relay-server:dev)This is on top of the already-merged
ee63f5d45(Docker-side pruning of the duplicateui-webcopy andmysql-client/non-matching-arch files, reflected in the245.2 MBbaseline above) — this fix additionally coversapps/stack's native self-host installers, which consume the same underlying artifact but don't go through that Docker-side pruning at all.Verification
buildServerBinaryArtifactPayload.prismaEnginePrune.test.ts(4 cases) — RED confirmed before implementation, GREEN after.packages/cli-commoncomponentArtifactssuite: all passing, no regressions.yarn tsc -p packages/cli-common).sqliteprovider boots and servesHTTP 200.mysqlprovider correctly locates and loads its query engine (fails only on the fakeDATABASE_URLconnection, as expected — no "Could not locate the Query Engine" error).happierdev/relay-server:devimage and diffed its real OCI manifest layer sizes against a locally-built image with this fix applied (same Dockerfile recipe, pushed to a local registry) — not justdocker images, which shows uncompressed and can double-count shared layers.Test plan
cli-commontest suite greenNote
Prune foreign-platform and unreachable Prisma artifacts during server binary payload build
pruneServerPrismaArtifactsForTargetinbuildServerBinaryArtifactPayload.tsto remove non-target native engine binaries fromnode_modules/.prisma/clientand eachgenerated/*-clientdirectory..mapsourcemap files fromnode_modules/@prisma/client/runtime.buildServerBinaryArtifactPayload; missing directories are treated as no-ops.Macroscope summarized 2f6e872.
Summary by CodeRabbit