Repository navigation
chore(deps): upgrade Effect to stable 4.0.0 - #14563
juliusmarminge wants to merge 2 commits into
Conversation
Effect 4.0.0 drops the `effect/unstable/*` export paths, so every import
moves to its flattened `effect/<area>` path (`httpapi` becomes `http-api`).
`effect/Encoding` is split into `effect/encoding/{Base64,Base64Url,Hex}`.
Behavior changes picked up from rc.116 through 4.0.0:
- `HttpRouter.serve` builds the app in a private memo map, so the server's
`TracerDisabledWhen` predicate is now provided to the served layer.
- `Scope.close` requires a closeable scope: SSH tunnels take their entry
scope explicitly and the Codex runtime forks a child of the caller's.
- `Effect.partition` returns successes first; `Stream.scan` takes a lazy
initial state; `SchemaGetter.onSome`/`.compose` are replaced.
- `Schema.brand` requires a single concrete brand key.
The effect patch is re-ported: MCP DELETE now lives in the HTTP protocol
layer against the new stateful runtime, and the missed-pong tolerance and
RpcClient hooks keep upstream's `SocketReadError` ping-timeout reason.
The relay still pins alchemy 2.0.0-beta.79, which imports the removed
paths; it needs alchemy beta.80 before it loads again.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This is a 703-file Effect major-version migration that includes runtime tracing, scope-lifecycle, parsing, and authentication-path changes rather than only import renames. An unresolved High-severity finding also identifies unbounded ACP parser buffering that can exhaust server memory. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Effect 4.0.0's ndJsonRpc parser skips lines that are not JSON, so a malformed agent line no longer terminated the ACP session. Split frames here and decode each line with the strict jsonRpc codec. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| const makeStrictNdJsonRpcParser = () => { | ||
| const codec = RpcSerialization.jsonRpc().makeUnsafe(); | ||
| const decoder = new TextDecoder(); | ||
| let buffer = ""; | ||
| return { | ||
| decode: (bytes: Uint8Array | string): ReadonlyArray<unknown> => { | ||
| buffer += typeof bytes === "string" ? bytes : decoder.decode(bytes, { stream: true }); | ||
| const lines = buffer.split("\n"); | ||
| buffer = lines.pop() ?? ""; | ||
| return lines.flatMap((line) => codec.decode(line)); |
There was a problem hiding this comment.
🟠 High src/protocol.ts:91
An agent that never sends \n causes buffer to grow without limit, so the server eventually exhausts its memory instead of terminating the session. Add the 16 MiB incomplete-frame cap used by ndJsonRpc() and throw when it is exceeded.
const makeStrictNdJsonRpcParser = () => {
+ const MAX_BUFFERED_FRAME_SIZE = 16 * 1024 * 1024;
const codec = RpcSerialization.jsonRpc().makeUnsafe();
const decoder = new TextDecoder();
let buffer = "";
return {
decode: (bytes: Uint8Array | string): ReadonlyArray<unknown> => {
buffer += typeof bytes === "string" ? bytes : decoder.decode(bytes, { stream: true });
+ if (buffer.length > MAX_BUFFERED_FRAME_SIZE) {
+ throw new Error("Maximum incomplete JSON-RPC frame size exceeded");
+ }
const lines = buffer.split("\n");🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/effect-acp/src/protocol.ts around lines 91-100:
An agent that never sends `\n` causes `buffer` to grow without limit, so the server eventually exhausts its memory instead of terminating the session. Add the 16 MiB incomplete-frame cap used by `ndJsonRpc()` and throw when it is exceeded.
|
Thanks for working on this. We merged the orchestrator V2 rewrite in #2829, and we are closing this PR as part of that transition. This PR upgrades Effect across 703 pre-V2 files, modifying 34 files removed by the rewrite and no orchestration-v2 files. The newly merged server must be included in the dependency migration and validation. Rebuild the upgrade on current main. Sorry for the extra work this creates. If the change is still needed on V2, please rebuild it on current main, verify it there, and open a new PR linking back here. We're closing the current implementation without assuming the underlying request is resolved. |
Effect 4.0.0 is out as the first stable v4 release. We were on
4.0.0-rc.115. Between the two, Effect removed theeffect/unstable/*export paths and theeffect/Encodingmodule, and changed a few APIs we depend on.Changes
effect/unstable/*now use the flattenedeffect/<area>paths.httpapiis nowhttp-api, andarbitrary/Arbitraryis noweffect/Arbitrary.effect/Encodingis replaced byeffect/encoding/{Base64,Base64Url,Hex}.HttpRouter.servenow builds its app in a private layer memo map. Because of that, the server-wideTracerDisabledWhenpredicate, which was merged intomakeRoutesLayer, stopped applying, and the browser OTLP proxy route was traced again.server.test.tscaught this. A newwithUntracedRequestshelper provides the predicate to the served layer, and both the server and the test use it.Scope.closenow only accepts a closeable scope.closecan still end it.Effect.partitionnow returns successes first.Stream.scantakes a lazy initial state.SchemaGetter.onSomeis replaced bytransformEffect, and.composebySchemaGetter.compose/SchemaTransformation.composeTransformation.Schema.brandrequires a single concrete brand key.Effect.orElseSucceednow passes the error, which needed a type annotation on the WSL test stub.getSetCookieguard carry over. The ping timeout now uses upstream'sSocketReadErrorreason.@effect/vitest. The patch is still just thevite-plus/testimport swap. pnpm rejectscatalog:in the@effect/vitestpackageExtensionsentry once it re-resolves, sovite-plusis pinned there literally.Known gap: relay
infra/relaystill usesalchemy@2.0.0-beta.79, the latest published version. It and its@distilled.cloud/*dependencies import the removedeffect/unstable/*paths, so the relay will not typecheck, test or deploy until alchemy publishes beta.80. Alchemy main already supports the 4.0.0 module layout. Its preview tarballs depend on URL-pinned distilled packages, which ourblockExoticSubdepspolicy rejects, so this PR does not pin them. The follow-up is to bump alchemy once beta.80 is on npm..repos/effect-smolwill be synced in a separate refs PR, as before.Verification
tsc --noEmitis clean for server, web, desktop, mobile, contracts, shared, client-runtime, ssh, tailscale, effect-acp, effect-codex-app-server, scripts and oxlint-plugin-t3code.server.test.ts, including the OTLP untraced testMcpHttpServer.test.ts, including MCP session DELETEsession.test.ts, including missed-pong tolerance🤖 Generated with Claude Code