chore: retarget effect-utils main - #1334
Closed
schickling-assistant wants to merge 20 commits into
Closed
Conversation
Contributor
PR preview
Report historyPR 1334 · 2026-06-25 12:12 UTC
PR 1334 · 2026-06-25 11:27 UTC
PR 1334 · 2026-06-24 13:36 UTC
PR 1334 · 2026-06-23 19:51 UTC
PR 1334 · 2026-06-23 18:15 UTC
PR 1334 · 2026-06-22 17:07 UTC
PR 1334 · 2026-06-22 16:53 UTC
PR 1334 · 2026-06-22 16:10 UTC
PR 1334 · 2026-06-22 14:09 UTC
PR 1334 · 2026-06-22 13:48 UTC
PR 1334 · 2026-06-22 12:50 UTC
PR 1334 · 2026-06-22 12:21 UTCNo previews were published for this commit. PR 1334 · 2026-06-22 11:57 UTCNo previews were published for this commit. PR 1334 · 2026-06-22 09:10 UTCNo previews were published for this commit. PR 1334 · 2026-06-21 12:07 UTC
|
schickling
marked this pull request as ready for review
June 22, 2026 07:45
schickling-assistant
changed the title
The pure-pnpm install used by the effect-utils retarget does not create node_modules/.bin entries, so binaries must be resolved via the scripts/bin PATH shims. The prior shim pass covered vitest/vite/madge/playwright but missed: - astro (docs build) — add scripts/bin/astro shim and call bare `astro` instead of `pnpm astro` in docs build/dev (pnpm <bin> bypasses the PATH shim). - wrangler (cloudflare example build) — add scripts/bin/wrangler shim. - perf test invoked `pnpm playwright`; switch to bare `playwright` so the PATH shim resolves it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
The effect-utils pnpm install task copied the genie-generated (read-only) pnpm-install-contract.json into the task cache with a plain `cp`, so a retried CI job running pnpm:install twice failed with `cp: ... Permission denied`. Re-pin effect-utils to ff93cf0f (overengineeringstudio/effect-utils schickling/2026-06-22-pnpm-contract-cp-fix, branched from the previously pinned df396b6) which makes that copy idempotent. Ref stays `main` to satisfy the default-ref source policy; only the locked commit/narHash move. Validated locally: a cache-miss pnpm:install with a pre-existing read-only contract cache copy now succeeds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
The generated contract embeds the effect-utils member commit, so re-pinning to ff93cf0f made the committed artifact stale and failed `lint:check:genie`. Regenerate it to match the new pin. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
The pure-pnpm install (always --ignore-scripts, isolated store) breaks npm
packages that ship platform-specific prebuilt binaries:
- d2 (astro-d2 docs diagrams): the terrastruct-d2-bin postinstall is skipped,
so provide the matching d2 0.7.1 from Nix on PATH instead.
- @parcel/watcher: the published @livestore/devtools-vite bundles it and does a
runtime require('@parcel/watcher-<platform>') that cannot resolve from the
isolated store. Hoist the platform packages publicly so the bundled native
loader resolves them in example/integration vite builds.
Validated locally: example vite build succeeds and the docs astro build gets
past the d2 integration setup.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
The published @livestore/devtools-vite bundles @parcel/watcher and resolves its
prebuilt platform package at runtime via require('@parcel/watcher-<platform>').
Under the pure-pnpm global virtual store that package is not reachable from the
bundle's location, so example and integration vite builds fail with
"Cannot require module ./build/Release/watcher.node".
The earlier publicHoistPattern attempt does not help: GVS resolves from each
package's own isolated closure, not a project-root hoist. Instead, inject the
@parcel/watcher platform packages directly into devtools-vite's dependency
closure via packageExtensions (optional + os/cpu-scoped), so the bundled
require resolves them. The lockfile now records these under the devtools-vite
snapshot, so a frozen install places the matching binary deterministically.
Replaces the ineffective publicHoistPattern from the previous commit.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
The standalone examples declare @playwright/test 1.59.1, but their e2e tests run against the Nix-provided Playwright browsers built for the catalog version (1.61.0, as used by the integration suite). The mismatch made examples:test fail with "Executable doesn't exist at .../chrome-headless-shell-1217". Override @playwright/test to the catalog 1.61.0 for the monorepo install so the example tests resolve the matching browser revision. The examples' own package.json versions are left untouched so they remain standalone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
The standalone examples are hand-maintained (not genie-managed) so they stay copyable, which let their dependency versions silently drift from the monorepo catalog — surfaced when examples:test failed on a Playwright browser-version mismatch (1.59.1 vs the Nix browsers built for the catalog's 1.61.0). Rather than paper over it with a monorepo-only override, align the examples to the catalog directly and add a guard: - Bump every drifting example dep to its catalog version (react 19.2.7, vite 8.0.16, typescript 6.0.3, @playwright/test 1.61.0, @vitejs/plugin-react 6.0.2, @tanstack/* 1.17x, tailwindcss 4.3.1, @types/*, ...). Validated example builds with the major bumps (vite 7->8, TS 5->6) locally. - Remove the interim @playwright/test workspace override (no longer needed). - Add `mono examples check-catalog` (CI gate, wired into lint:full) and `mono examples align-catalog` (fixer). The check reads the example manifests against the catalog without adding any genie files to the example dirs, so the examples stay standalone and copyable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
Collaborator
|
@codex pls look for regressions or behavior/logic changes |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/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: f38188c9a4
ℹ️ 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".
…down-ready Aligning the examples to the catalog's vite 8 surfaced a hard build failure in web-todomvc-redwood: rwsdk/vite (RedwoodSDK) is not compatible with vite 8's Rolldown bundler, and the published @livestore/devtools-vite still uses vite-7 config APIs. vite 8 is not yet usable across the example plugin ecosystem. Keep vite (7.3.1) and @vitejs/plugin-react (5.1.2) on the working line in the examples and allowlist them in `mono examples check-catalog`, with a documented reason and removal condition. Every other example dep stays aligned to the catalog (react, typescript, @playwright/test, @tanstack/*, tailwindcss, ...). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
`CloseEvent` is a DOM global and is undefined in non-DOM runtimes (e.g. the Node-based sync-provider DO RPC path that uses this socket protocol). A malformed/undecodable frame would hit the catch block and throw a ReferenceError on `defect instanceof CloseEvent` before the intended ClientProtocolError could be emitted. Guard with `typeof globalThis.CloseEvent === 'function'` first. Addresses Codex review on PR #1334. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V6d1Z5y4Cuh76LQN75TXQ7
schickling
approved these changes
Jun 22, 2026
…y-liveness
Repin release/devtools-artifact.json to the fixed DevTools artifact
(dt-20260623-4684daf4). The new @livestore/devtools-vite externalizes
@parcel/watcher (bare `import * as ... from "@parcel/watcher"`) and declares
it as a dependency, instead of the old runtime
`require("@parcel/watcher-<platform>")` that crashed the dev server under
pure-pnpm GVS.
release:devtools-artifact:certify-liveness repacks the pinned source artifact
and `cp -a`-replaces tests/integration/node_modules/@livestore/devtools-vite
with it. That `cp -a` severs the package from its own dependency closure, so the
pnpm-workspace.yaml `packageExtensions` injection does not survive it. Add
@parcel/watcher (^2.5.0) as a devDependency of @local/tests-integration so the
meta package resolves directly from tests/integration/node_modules under GVS,
which is necessary for the repacked artifact's dev server to start.
Keep the @livestore/devtools-vite `packageExtensions` block: the examples and
integration-playwright jobs still install the OLD bundled npm package
(0.4.0-dev.25), so they still need it. It is removable once the fixed
devtools-vite is republished to npm (not just as a source artifact).
Document the cross-repo DevTools artifact origin and the
"devtools-vite source change -> cut new overeng artifact -> repin -> snapshot"
runbook in contributor-docs/release-workflows.md, linking the overeng VRS in
context/devtools-artifact-release/.
Refs #1334
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E58y9wCig7yrao1ZCVu1bW
The release `validate-release-plan` certify-liveness step ran raw `playwright test web.play.ts` but never started the vite dev server (its webServer config is commented out; the server is normally started by run-tests.ts's viteDevServer). web.play.ts therefore connected to a dead port and failed with ERR_CONNECTION_REFUSED — a pre-existing gap since 0c63639. The integration-playwright (devtools) CI job passes precisely because it routes through run-tests.ts's devtoolsTest, which starts the server. Route the certify through that proven path: - run-tests.ts: add a `--web-only` option to the `devtools` command that scopes the run to web.play.ts (the certify only needs the web test and lacks browser-extension.play.ts's chrome-extension setup). - devenv.nix: invoke `bun run-tests.ts devtools --mode headless --web-only` instead of raw playwright, so viteDevServer starts. - devenv.nix: blank VITE_OTEL_EXPORTER_OTLP_ENDPOINT for the run (matches ci.yml's "Disable in Vite"). Now that the page actually loads it would otherwise POST traces to the devenv-defaulted localhost:4318, which has no collector in validate-release-plan and surfaces a console error that effect-playwright turns into a SiteError failure. - test-commands.ts: pass webOnly: false at the `integration all` callsite to satisfy the new required arg. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E58y9wCig7yrao1ZCVu1bW
…sure The certify-liveness harness replaced the installed @livestore/devtools-vite pnpm symlink with a real directory (cp -a of the repacked tarball) at the logical location. A real dir there is no longer realpathed into the pnpm global virtual store, so the plugin's runtime deps (@livestore/adapter-web, @livestore/utils) resolved by walking up tests/integration/node_modules and bound to raw workspace TS source instead of the pinned dev.25 dist — a *severed* closure that diverges from what ci.yml's passing test-integration-playwright (devtools) exercises, and silently breaks the served DevTools panel boot (web.play.ts's `window.__debugLiveStore.default` gate timing out). Keep the symlink semantics instead: build a writable copy of the GVS integrity dir (whose node_modules holds the dependency closure), rewrite the copied symlinks to absolute store targets so they survive the copy, overlay only the repacked dist + version-stamped package.json, and point the installed symlink at that copy. The plugin then runs the REPACKED dist while resolving its deps through an INTACT closure (GVS dist), matching the passing install. A bun assertion verifies every dep resolves into the store, guarding against future re-severance. Also add a temporary, env-gated (LIVESTORE_DEVTOOLS_DIAGNOSTICS) diagnostic at the panel boot gate that dumps the booted __debugLiveStore stores and probes the plugin-served panel URL, so a non-green certify run is still diagnosable. Playwright trace/screenshot are already on and uploaded as the validate-release-plan certification artifact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182BfmAryicy4YUeUEMVn92
…certify The first intact-closure pass relocated the plugin into an isolated /tmp closure that resolved @livestore/* + vite correctly but LOST Node's walk-up to the hoisted tests/integration/node_modules/@parcel/watcher. plugin.js imports @parcel/watcher directly (bundled), so the vite dev server (node) crashed at config load with ERR_MODULE_NOT_FOUND (livestore run 28101195536: closure verified intact, then watcher not found). Re-add @parcel/watcher to the copied closure, symlinked to the same GVS realpath the hoisted entry points at — it native-loads its platform .node from its OWN integrity closure, so the symlink location is irrelevant. Extend the closure assertion to also require @parcel/watcher and to native-load it (typeof subscribe === 'function'), and run the assertion under `node` rather than `bun`: the dev server importing plugin.js runs under node, and node provides the libstdc++ the watcher addon links against (bun in some runners does not, which would false-fail the native-load check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182BfmAryicy4YUeUEMVn92
…-effect-utils-pr805 # Conflicts: # .github/workflows/deploy-prod.yml # .github/workflows/release.yml # docs/package.json # packages/@local/astro-tldraw/package.json # packages/@local/astro-twoslash-code/package.json # pnpm-lock.yaml
Temporary diagnostic (gated on LIVESTORE_DEVTOOLS_DIAGNOSTICS) to pinpoint why the DevTools panel's `.default` store never boots in the release certify run. Attaches console/pageerror/response listeners on the app page before `goto`, and on the `.default` boot-gate timeout dumps the browser console, the filtered network outcomes for the plugin-served panel assets + RPC, an in-browser fetch probe of `/_livestore/static/index.js` + `.css`, and the page frames. The prior `page.request.get` probe was a server-side fetch (always 200) and could not tell us whether the browser's real asset loads 404 or the panel JS throws. Remove once the certify is reliably green. See tmp/devtools-vite-followup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0182BfmAryicy4YUeUEMVn92
Collaborator
Author
|
Closing as fully superseded. effect-utils is already retargeted to the Posted on behalf of @schickling
|
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
2 participants
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
LiveStore's effect-utils integration had moved through an upstream PR pin, which left the review surface and generated artifact handling stale once effect-utils was available from the default
mainref.Goal
Retarget LiveStore's effect-utils inputs back to default/main authority refs while keeping the resolved locks on the intended main commit, and make the generated pnpm install contract clearly reviewable as generated output.
Decisions
megarepo.kdlusesoverengineeringstudio/effect-utils#main;devenv.yamlusesgithub:overengineeringstudio/effect-utils/mainandgithub:overengineeringstudio/effect-utils/main?dir=nix/playwright-flake.df396b6599157e7861a80d333e80bb2ebdfbe82f.pnpm-install-contract.json, including source files, regeneration/check commands, an input fingerprint, and the effect-utils lock identity..gitattributesso GitHub collapses noisy generated diffs.Verification
rg -n "schickling/2026-06-19-deps|0febc86|c733da81|582417" -S .-> no matchesdevenv tasks run genie:check --mode before --no-tui-> passeddevenv tasks run mr:source-policy-check --mode before --no-tui-> passeddevenv tasks run mr:lock-sync-check --mode before --no-tui-> passeddevenv tasks run lint:full:fix --mode before --no-tui-> passed, including pre-commitcheck-quick1fe78a209Complexity
No new runtime complexity. The added generator logic records existing semantic inputs and computes a deterministic fingerprint for the committed projection artifact.
Concerns
mr:source-policy-checkverifies default-ref usage; CI hasVERIFY_REACHABLE=0, so lock commit reachability is represented by the lock data rather than proven by that job.pnpm-install-contract.jsonis a committed boundary artifact.Friction & bottlenecks
devenvshell evaluation took several minutes on cold runs while building/downloading Genie and megarepo tooling.import.meta.urlwas not suitable for readingmegarepo.lock; the generator now reads fromprocess.cwd().Follow-ups
References
df396b6599157e7861a80d333e80bb2ebdfbe82fPosted on behalf of @schickling
agent_nameagent_session_idagent_toolagent_tool_versionagent_runtimeagent_modelruntime_profileskills_manifestworktreemachinetooling_profile