refactor(effect-lsp)!: adopt effect-utils full-strict Effect-LSP gate (remove errors-only override) - #1410
Merged
Conversation
Contributor
PR preview
Report historyPR 1410 · 2026-07-19 11:58 UTC
PR 1410 · 2026-07-19 08:57 UTC
PR 1410 · 2026-07-18 20:11 UTC
PR 1410 · 2026-07-18 10:43 UTC
PR 1410 · 2026-07-18 10:04 UTC
PR 1410 · 2026-07-18 08:37 UTC
PR 1410 · 2026-07-17 15:56 UTC
PR 1410 · 2026-07-17 15:05 UTC
PR 1410 · 2026-07-17 14:03 UTCNo previews were published for this commit. PR 1410 · 2026-07-15 22:41 UTCNo previews were published for this commit. |
schickling-assistant
force-pushed
the
schickling-assistant/2026-07-15-bump-effect-utils
branch
from
July 16, 2026 15:04
ddb58d5 to
9e9847a
Compare
Base automatically changed from
schickling-assistant/2026-07-15-bump-effect-utils
to
main
July 17, 2026 12:19
schickling-assistant
force-pushed
the
schickling-assistant/2026-07-16-bd-capstone
branch
from
July 17, 2026 13:32
67072cd to
047818b
Compare
schickling-assistant
marked this pull request as ready for review
July 17, 2026 15:57
Collaborator
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5aba317e1
ℹ️ 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".
schickling-assistant
force-pushed
the
schickling-assistant/2026-07-16-bd-capstone
branch
5 times, most recently
from
July 18, 2026 19:44
0a810a6 to
f29fa86
Compare
schickling
approved these changes
Jul 19, 2026
… (remove errors-only override) Removes LiveStore's temporary errors-only Effect-LSP override in genie/repo.ts so the inherited effect-utils effectDiagnosticsGate applies in full: Effect errors, warnings, AND suggestions now all gate `tsgo --build`, not just errors. The LiveStore-specific `allowedDuplicatedPackages: ['@livestore/utils']` is folded into the base plugin config so it survives the override removal. This is the capstone of the #811 Effect-LSP burndown; it lands on top of the merged burndown work (#1407 mechanical, #1408 schema, #1409 semantic) which cleared all pre-existing advisory diagnostics. Regenerated all 28 package tsconfigs via genie so the full gate (ignoreEffectWarnings/Suggestions = false) is reflected in the committed generated config. Acceptance: `tsgo --build --force tsconfig.dev.json` builds all projects and exits 0 with zero Effect diagnostics under the full gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KNpusCebCSBEGLzCWnJfau
…e full gate in ClientSessionSyncProcessor test The synthetic `Exit.fail(new Error(...))` in ClientSessionSyncProcessor's shutdown test (added on main via #1439) is advisory under the errors-only gate but fatal once this capstone removes the override. It's a throwaway test signal fed to Scope.close, so a justified globalErrorInEffectFailure:off is the right call — a tagged error adds no value here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KNpusCebCSBEGLzCWnJfau
…Error under the full gate The trusted-PR-snapshot flow added to scripts/src/commands/release.ts on main via #1458 constructs global `new Error(...)` values in the Effect failure channel (globalErrorInEffectFailure / globalErrorInEffectCatch). These are advisory under the errors-only gate but fatal once this capstone removes the override. Reroute the two validation failures and two tryPromise catch callbacks through the file's existing `ReleaseError` tagged error (adding an optional `cause` field so the wrapped filesystem errors survive), and drop the redundant `Effect.fail` wrapper now that a yieldable error is yielded directly. Behavior- neutral. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KNpusCebCSBEGLzCWnJfau
schickling-assistant
force-pushed
the
schickling-assistant/2026-07-16-bd-capstone
branch
from
July 19, 2026 11:37
5a9edb1 to
9a4345d
Compare
schickling
enabled auto-merge (squash)
July 19, 2026 14:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The effect-utils bump adopted
@effect/language-servicewitheffectDiagnosticsGate = { warnings: true, suggestions: true }, which failstsgo --buildon every Effect error, warning, and suggestion — not just errors. That surfaced ~406 pre-existing advisory diagnostics across the LiveStore tree, so the bump PR (#1397) shipped a temporary errors-only override ingenie/repo.ts(flippingignoreEffectWarningsInTscExitCode/ignoreEffectSuggestionsInTscExitCodetotrue) to keep the tree green while the burndown landed separately.Approach
This is the capstone of the #811 Effect-LSP burndown. It:
duplicatePackageconfig (allowedDuplicatedPackages: ['@livestore/utils']) + 36 mechanical fixes (incl.schemaSyncInEffect→encode*Effect+orDie)Schema.Number→Schema.Finite(+ 1 justifiedfield-defs.ts:268carve-out)preferSchemaOverJson, tagged-error hygiene) + justified:offforanyUnknownInErrorContextgenie/repo.tsso LiveStore inherits effect-utils' fulleffectDiagnosticsGateunchanged. The LiveStore-specificallowedDuplicatedPackages: ['@livestore/utils']is folded into the base plugin config so it survives the override removal (otherwise 183duplicatePackagediagnostics would return).genie:run— every generated tsconfig now carriesignoreEffectWarningsInTscExitCode: false/ignoreEffectSuggestionsInTscExitCode: false(full gate) while keepingallowedDuplicatedPackages.Merge conflict resolution
Two
.test.tsfiles were touched by both bd-mechanical and bd-semantic; both were resolved preserving both independent fixes (and validated by the now-fatal gate building clean):packages/@livestore/common/src/sync/syncstate.test.ts— kept mechanical'syield* Schema.encodeUnknownEffect(...).pipe(Effect.orDie)(schemaSyncInEffect) and semantic'sjsonParse(jsonStringify(...))helper (preferSchemaOverJson).packages/@local/astro-twoslash-code/src/cli/snippets.watch.test.ts— kept mechanical's consolidated block-level}).pipe(Effect.orDie)and semantic's module-scopedjsonStringifyhelper.Acceptance test (the point of this PR)
With the override removed and the full gate active:
tsgo --build --force tsconfig.dev.json→ builds all 32 projects, exits 0, zero Effect diagnostics.devenv tasks run ts:check→ green (exit 0).devenv tasks run check:quick→ green (exit 0).Every remaining Effect warning/suggestion is either fixed or carries a justified inline
:off.Justified
:offexemptions (reviewer audit)Carried in from the burndown branches; all inline-annotated with rationale:
anyUnknownInErrorContext(#1409 — dynamic dispatch / vendored fork / regression tests):packages/@livestore/common-cf/src/do-rpc/server.ts— lines 88, 99, 135, 149, 172, 256 (Schema.toCodecJsonon erased dynamic schemas /Rpc.Handler.handlerdynamic dispatch; context provided at runtime)packages/@livestore/livestore/src/effect/LiveStore.test.ts— lines 32, 40 (regression test Store.Tag: StoreContextId vs StoreTagClass R channel mismatch #1103 intentionally widens channels tounknown)packages/@livestore/utils/src/effect/Subscribable.ts— line 49 (vendored fork;unknownis structural over genericA, E, R)schemaNumber(#1408 — SQLite REAL columns legitimately store Infinity/NaN):packages/@livestore/common/src/schema/state/sqlite/db-schema/dsl/field-defs.ts— line 268 (public DEFAULT codec must accept non-finite;Schema.Finitewould wrongly reject)preferSchemaOverJson(#1409 — committed/human-inspected JSON must stay indented; Schema's JSON codec emits compact):scripts/src/commands/docs.ts— line 708scripts/src/commands/release.ts— lines 188, 276scripts/src/commands/update-deps.ts— line 289Notes
!) reflects the CI-contract change: warnings/suggestions now gate the build.pnpm-lock.yaml/repos/changes.🤖 Generated with Claude Code
Posted on behalf of @schickling
agent_nameagent_session_idagent_toolagent_tool_versionagent_runtimeagent_modelruntime_profileskills_manifestworktreemachinetooling_profile