Skip to content

Make fresh worktrees build with two commands - #1830

Closed
michielbdejong wants to merge 2 commits into
developfrom
claude/fresh-worktree-setup
Closed

michielbdejong wants to merge 2 commits into
developfrom
claude/fresh-worktree-setup

Conversation

@michielbdejong

Copy link
Copy Markdown
Contributor

A fresh worktree now needs only the two documented commands:

cd browser && pnpm install && pnpm run -r build
cd ../server && cargo build

What was wrong and what changed

1. build:wasm failed with "No package found for binary run". Worktrees under .claude/worktrees/ sit inside the main checkout. Cargo reads every .cargo/config.toml up the directory tree and concatenates array values, so the alias bin = ["run", "--package", "cargo-bin", "--"] turned into run -p cargo-bin -- run -p cargo-bin -- wasm-pack. The shim then looked for a binary called run. A string alias doesn't fix this, because cargo refuses to merge a string with the parent checkout's array. So the alias is gone, and build:wasm calls cargo run -q --package cargo-bin -- wasm-pack directly. On first use it installs wasm-pack into that worktree's own .bin/ (about 35s), with no dependency on the main checkout's path.

2. Builds rewrote src/chunks/Website/runtime/{search-view.html,website-runtime.min.js}. The generator was already deterministic. The committed copies were stale: websiteRuntime.ts had gained the proxy* ops without anyone regenerating them. Checking them in invites that drift again, so they are now gitignored. A small vite plugin generates them in its config hook, which runs for vite dev, vite build and vitest alike, and again on HMR of the runtime sources. Every consumer goes through vite: server/build.rs (pnpm run build), Dagger jsBuild/jsTest, build:tauri, and the release workflows. The exception is tsc, which resolves ?raw/?url through vite/client's wildcard modules. pnpm typecheck passes with the files deleted, so Dagger's jsTypecheck (which doesn't build data-browser) is unaffected.

3. Store-backed vitest failed under Node 26. Node 25+ puts Web Storage on globalThis, and without --localstorage-file its localStorage is undefined. Vitest's jsdom environment only copies window keys that the global lacks, so jsdom's storage never reached window. The new data-browser/src/test-setup.ts restores jsdom's localStorage/sessionStorage when they are missing. Unlike --no-experimental-webstorage, it keeps working if Node drops that flag. Supported: Node 22 (CI) through 26.

4. pnpm -r run build raced lib against cli. @tomic/lib had a devDependency on @tomic/cli (only for the ad-generate bin in generate-ontologies), while cli depends on lib. pnpm breaks cycles arbitrarily, so cli's tsc could run while lib was rebuilding. The devDependency is removed and generate-ontologies runs cli from ../cli directly. The root build script is now plain pnpm run -r build, without the lib-first workaround.

5. Docs. AGENTS.md "Fresh Worktrees" lists the two commands and the workarounds that are no longer needed. It also corrects a stale claim: the pre-commit hook stubs assets_tmp and skips the JS build, so it needs only pnpm install.

The lockfile diff also drops the orphaned @automerge/* entries and a jsdom peer from lib/plugin/react's vitest resolution. Nothing in those packages uses jsdom.

Verification

  • Fresh git worktree add from this branch: pnpm install && pnpm run -r build exits 0 (1:51, including the wasm-pack install and WASM build). lib finished before cli started. git status stayed clean afterwards.
  • In a worktree nested in the main checkout, the old cargo bin wasm-pack --version reproduced "No package found for binary run". pnpm build:wasm now passes there.
  • cargo build in server/ of the fresh worktree: exit 0 (3:44; build.rs ran the full JS build, including this change's vite plugin).
  • A normal commit that touched a .rs file in the fresh worktree passed the pre-commit hook (lint snapshot plus clippy), then was reverted.
  • vitest on Node 26.8.1 and 22.23.2: data-browser 164 files / 1309 tests pass on both (4 recentResources tests failed on 26 before). lib 1013 tests pass on both.
  • pnpm typecheck and pnpm lint in data-browser pass.
  • These commits went through the normal pre-commit hook, including the clippy snapshot (the change touches tools/cargo-bin/Cargo.toml).

🤖 Generated with Claude Code

Michiel de Jong and others added 2 commits September 25, 2026 15:36
- build:wasm: drop the `cargo bin` alias. Cargo concatenates array config
  values from every parent `.cargo/config.toml`, so in a worktree nested in
  the main checkout the alias doubled into `run -p cargo-bin -- run ...`
  ("No package found for binary run"). The script now runs the shim package
  directly, which installs wasm-pack into the worktree's own `.bin/`.
- Website runtime bundles are gitignored and generated by a vite plugin on
  every vite / vitest start, instead of committed copies that had drifted
  from their sources and got rewritten by every build.
- data-browser vitest: a setup file restores jsdom's localStorage and
  sessionStorage, which Node 25+'s built-in Web Storage shadows.
- Remove @tomic/lib's devDependency on @tomic/cli. The cycle let pnpm build
  cli concurrently with lib; `pnpm run -r build` is now ordered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@michielbdejong michielbdejong added browser ci Github actions / e2e tests / build / etc devex More PRs per minute labels Sep 25, 2026

joepio commented Sep 25, 2026

Copy link
Copy Markdown
Member

I left this out of batch #1853. Merged onto develop, data-browser tsc --noEmit fails:

vite.config.ts(14,37): error TS7016: Could not find a declaration file for module './scripts/build-website-runtime.mjs'. '.../scripts/build-website-runtime.mjs' implicitly has an 'any' type.

A build-website-runtime.d.mts next to the script (or converting it to .ts) would fix it. Once that's done it goes into the next batch.


Generated by Claude Code

@michielbdejong

Copy link
Copy Markdown
Contributor Author

Merge-risk triage (2026-09-28). This is a read-only review of this PR's own diff. The rule it applies: nothing that works today may break for existing users or clients, and new behaviour is added alongside the old. The stack is being rebased onto develop as one chain before merging, so line references are against the current stacked base.

Verdict: SAFE

It only changes build and dev scripts. The generated Website runtime files are now gitignored and rebuilt by a vite plugin that every consumer runs through (dev, build, vitest, server/build.rs, Dagger, Tauri). No caller in develop uses the removed cargo bin alias.

Note: Main CI doesn't run on PRs (main.yml triggers on develop pushes, tags and manual dispatch), so a workflow_dispatch on this branch before merging would be cheap insurance. #1834 calls cargo bin wasm-pack and needs a one-line change to go with this. Stale comment: browser/.npmrc:20.

🤖 Generated with Claude Code

joepio added a commit that referenced this pull request Sep 28, 2026
joepio added a commit that referenced this pull request Sep 29, 2026
… picker (#1842) (#1873)

Squash of Michiel's #1830 and #1842, plus a type declaration for build-website-runtime.mjs so Dagger's typecheck passes.
@joepio joepio closed this in #1873 Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

browser ci Github actions / e2e tests / build / etc devex More PRs per minute

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants