Skip to content

refactor(effect): name the lift's rejection channel; fold boundary settlements without an unknown failure - #517

Merged
ScriptedAlchemy merged 1 commit into
mainfrom
refactor/effect-lift-typing
Sep 4, 2026
Merged

ScriptedAlchemy merged 1 commit into
mainfrom
refactor/effect-lift-typing

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 4, 2026 •

Copy link
Copy Markdown
Owner

Summary

The two "small ones" from the Effect-conformance audit: the reconciler's momentary unknown on the fail channel, and the dev seam's identity lift inferring bare unknown at every call site. Behavior-preserving: no runtime path changes; every liftPromise/liftTry call site (82) type-checks unchanged.

What changed

packages/rsc-runtime/src/reconciler.ts:288–300 (audit row :293–295, Effect.tryPromise({ catch: (error) => error }) → unknownInEffectCatch). The audit suggested catch: toRuntimeError; that would be a behavior change: a boundary's rejection reason is data that renderErrorFrom (:130) reads — React rejects a boundary thenable with a { message, digest } object, and toRuntimeError would turn a non-Error object into new Error('[object Object]') and lose the digest-derived code. Instead waitSettledBoundary folds both settlements inside the promise (Effect.promise(() => thenable.then(ok, (error) => ({ boundary, error, ok: false })))), so the Effect can never fail and nothing unknown ever sits on the error channel — the reason travels as a field of SettledBoundary, as before. Return type narrows from Effect<SettledBoundary, Error> to Effect<SettledBoundary>.

packages/agent-bundle/src/effect/lift.ts (audit row :11–15). The audit proposed typing E as CodedError | DiagnosticError | unknown; that union collapses to unknown and any narrower union would be unsound — the lift is an identity by contract (typed dev errors and raw AbortSignal.reason values must cross the boundary untouched). So:

  • export type LiftedRejection = unknown names the channel: hovers and signatures now read Effect<A, LiftedRejection> with the doc explaining it is the raw thrown/rejected value that callers narrow (instanceof, isErrno, Effect.mapError into a typed dev error) rather than a shape to assume, and how the boundary maps whatever reaches it.
  • liftPromise's evaluate is now (signal: AbortSignal) => PromiseLike<A> — rc.112's own Effect.tryPromise contract (Effect.ts:969–973): the signal aborts when the fiber is interrupted, so a cancellable Promise API can take it directly. Zero-arg thunks are unchanged.
  • Module doc records why a bare Effect.tryPromise(fn) is not used (it wraps rejections in Cause.UnknownError).

Idioms and citations

Idiom Doc repos/effect/LLMS.md / repo section
No unknown / global Error on the fail channel; a value that is data stays in the success channel Error Management → Two Types of Errors, Expected Errors agent-patterns/effect-errors.md "What to avoid: unknown or global Error in the fail channel (unknownInEffectCatch)"
Effect.promise for a promise that cannot reject; tryPromise's try(signal) for interruption-aware lifts Resource Management → Scope → acquireRelease example (Effect.promise release), rc.112 Effect.promise / tryPromise signatures LLMS.md § Creating effects from common sources; docs/effect-conventions.md Stage 3 "Hurt": "Bare Effect.tryPromise(fn) wraps rejections in Cause.UnknownError; always route through src/effect/lift.ts"

Tests

  • New in packages/agent-bundle/tests/effect-boundary.test.ts (effect lifts): rejected typed errors and raw non-Error reasons are identity-preserved on the fail channel (Exit + Cause.squash), thrown values likewise via liftTry; the Promise edge rethrows typed errors as-is and wraps a non-Error per the mapping table; the lifted helper receives an AbortSignal that is not aborted while running and aborts when the host signal interrupts the fiber (AbortError at the edge).
  • Reconciler: the existing dispatcher boundary-error tests (emits a represented boundary error and still completes siblings, emits a replace or error for every boundary that settled before resnapshot) cover the fold; all 55 dispatcher/boundary tests green.
  • pnpm typecheck, pnpm lint green; pnpm test:unit green (3186 passed).

Changeset

skip-changeset: type-level and internal-structure only; no Promise-side value, message, or export changes.

Review status

No PR comments are posted by the author; every review thread is answered in this section.

Head Review Threads
a9e5459 Codex completed, no findings —

… settlements without an unknown failure

lift.ts: `LiftedRejection` names the identity lift's fail channel (the raw
thrown/rejected value, narrowed by callers), and `liftPromise` hands the
helper Effect's interruption AbortSignal per rc.112's tryPromise contract.

reconciler.ts: `waitSettledBoundary` folds a boundary's settlement inside
the promise instead of catching an `unknown` off the Effect channel; the
rejection reason stays data for `renderErrorFrom` (React's digest-bearing
objects), so it is deliberately not normalized through `toRuntimeError`.
@ScriptedAlchemy ScriptedAlchemy added the skip-changeset PR changes a publishable package but ships no observable change; changeset not required label Sep 4, 2026
@changeset-bot

changeset-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a9e5459

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-04T03:41:54.607038Z a9e5459 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pkg-pr-new

pkg-pr-new Bot commented Sep 4, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle@517
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@517
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@517

commit: a9e5459

@ScriptedAlchemy
ScriptedAlchemy merged commit 2727314 into main Sep 4, 2026
13 of 14 checks passed
@ScriptedAlchemy
ScriptedAlchemy deleted the refactor/effect-lift-typing branch September 4, 2026 04:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changeset PR changes a publishable package but ships no observable change; changeset not required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant