feat(sdk)!: dual ESM+CJS build with an exports map; take Veil off the root barrel

[nandal]

Addresses part 1 of #39 — the packaging half. Part 2 (the four smart-wallet client pieces) is untouched; see the note at the bottom.

What was wrong

The reporter's diagnosis holds, and the eager-instantiation question they flagged as unmeasured resolves the bad way:

sdk/src/veil/wasm/warden_wasm.js:356-358 is the nodejs wasm-bindgen target. It does

const wasmBytes = require('fs').readFileSync(wasmPath);
const wasmModule = new WebAssembly.Module(wasmBytes);
let wasmInstance = new WebAssembly.Instance(wasmModule, __wbg_get_imports());

at module-evaluation time. veil.ts:22 imports it top-level, and index.ts:144 re-exported Veil from the root barrel. So import { deriveReadingKeyFromPrfOutput } instantiated WebAssembly — and because fs.readFileSync does not exist in a browser, the root barrel didn't merely cost a CSP directive there, it could not load at all.

The CSP argument in the issue is the right frame for why this matters more than bundle size, and I've kept it in the code comments rather than only in this PR: a reading key is long-lived and its published half is write-once, so the origin deriving it is the single worst place to give up script-src 'self'.

What this does

Subpath	Formats	Reaches
.	ESM + CJS	everything except Veil
./crypto	ESM + CJS	crypto/ only — no ethers, no WebAssembly
./veil	CJS	Veil + the wasm; Node-only, which is what it always was

• Two tsc passes — dist/esm and dist/cjs — sharing tsconfig.base.json. src/veil is excluded from the ESM config, so the wasm-free property is structural, not a convention someone can undo with a stray re-export.
• exports map, module field, sideEffects naming the wasm glue as the one side-effecting module.
• Each tree gets a package.json type marker, so the root "type" stays unset and is not load-bearing. I chose that over "type": "module" at the root (which the issue suggested) purely to keep the blast radius off the repo's own tooling — consumers see the same thing either way.
• ./vectors/* and ./deployments/* are explicitly mapped. Worth flagging: an exports map is deny-by-default, so without these the map would have silently broken require("@bytesbrains/maktub-sdk/vectors/reading-key.json") — the deep path 0.1.0-dev.4's changelog told language ports to use.

Verification

Not asserted — checked. npm run verify:packaging runs as part of npm run build, so ci.yml and sdk-publish.yml both execute it with no workflow change:

✓ dist/esm is free of `warden_wasm` (53 files)
✓ crypto (esm) imports with no WebAssembly and exposes reading-key derivation
✓ root barrel no longer re-exports Veil
✓ ./veil loads and exposes veilSeal

The WebAssembly check traps the constructors before importing, so it covers the transitive graph rather than what a grep can see. It's scoped to ./crypto deliberately: the root entry pulls ethers → undici, and undici compiles its own llhttp wasm on Node. That's Node's HTTP stack, absent from any browser bundle, and trapping globally just reports it as our defect.

Beyond CI, I packed the tarball and installed it into a scratch consumer:

ok   esm  import root → MaktubClient
ok   esm  root has no veilSeal (moved to ./veil)
ok   esm  import ./crypto → HKDF derivation, no WASM instantiated
ok   cjs  require root → MaktubClient
ok   cjs  require ./vectors/reading-key.json (dev.4 contract held)
ok   cjs  require ./deployments/base-sepolia.json (10 contracts)
ok   esm  import ./veil → veilSeal (wasm loads)
ok   esm  ./crypto round-trips a payload

And bundled ./crypto for the browser with Vite to confirm the actual claim: 88.7 kB, zero hits for wasm / veil / WebAssembly / ethers.

sdk-publish.yml gains one gate: every exports target must be present in the tarball. verify:packaging checks the working tree and cannot see package.json#files, so a build correct locally could still publish a map pointing at unshipped files — an install-time ERR_PACKAGE_PATH_NOT_EXPORTED on a version number that's already burned.

Existing suite: 97 tests, 13 files, all passing; typecheck clean.

⚠️ One change beyond the issue — please look at this separately

@noble/curves and @noble/hashes were phantom dependencies. src/crypto/ imports them directly in six files, but neither was in dependencies — the lockfile had them as "dev": true, resolving only because ethers hoisted them into place.

I added them as real dependencies because otherwise this PR ships a ./crypto subpath that fails to resolve under pnpm's strict layout or Yarn PnP, which would undercut its whole purpose. The ranges (^1.2.0, ^1.3.2) resolve to exactly the versions ethers already pins, so nothing moved and no code changed — but it does mean the SDK now declares runtime dependencies where it previously declared none, and that's your call, not mine. It's a four-line revert in package.json if you'd rather handle it on its own.

Known, not fixed

crypto/aes.ts does await import("crypto") as a Node fallback for AES-GCM. Browser bundlers emit an externalization warning for it. I left it alone: the fallback is real (Node 18 predates stable global crypto.subtle, and engines says >=18), it's dynamic so it never enters the bundle, and silencing the warning properly means a browser export condition — a design change that doesn't belong in this PR.

Breaking

Veil moves off the package root. Named exports are unchanged, only the import path:

- import { veilSeal } from "@bytesbrains/maktub-sdk";
+ import { veilSeal } from "@bytesbrains/maktub-sdk/veil";

Deep paths into dist/ are also no longer reachable. Version bumped to 0.1.0-dev.5 with a migration note in the changelog.

On part 2

The four smart-wallet pieces do belong in this repo — the contracts they derive against are here (contracts/v3/wallet/, ERC-1167 Clones.cloneDeterministic + OZ WebAuthn/P256 + 4337 v0.7), deployments/base-sepolia.json already carries the factory and implementation, and vectors/ is the established home for byte-exact cross-language fixtures. The SDK has no wallet surface at all today, so it'd be net-new rather than a move.

Two things probably want deciding before anyone starts on it: whether the wallet layer belongs in the SDK or is app-layer under "simplicity in the protocol, complexity in the app", and — if it lands — that it should be its own ./wallet subpath, which this PR now makes cheap to add.

🤖 Generated with Claude Code

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[wrokin]

🤖 wrokin code review

Summary

No high-signal issues — implementation fully addresses #39’s browser-safe packaging (dual ESM/CJS, Veil off root barrel, exports gate).

Real issues (bugs / security / correctness)

None found. The diff correctly splits the SDK into separate trees, gates Veil behind a CJS-only subpath with appropriate sideEffects marking, and adds robust build-time and publish-time verification.

Notes

• The second part of [medium] SDK packaging blocks browser consumers: CJS-only barrel pulls Veil + WASM into every import #39 (smart-wallet logic porting) is not addressed; that is acceptable as the PR focuses on the packaging request.
• The verify-packaging script’s comment-aware grep and WebAssembly trap give strong defense in depth; no gaps observed.

_{Model: deepseek/deepseek-v4-pro · your key, your model (BYOK) · Context: #39}

[wrokin]

🤖 wrokin security audit

No security-relevant findings (injection, authz, secrets, unsafe deserialization, etc.) were identified in this pull request. The changes are structural and improve the security posture by isolating WASM-containing code from the default import path and providing a CSP-safe entry point, with appropriate verification gates. No remediation is needed.

_{Model: deepseek/deepseek-v4-pro · your key, your model (BYOK) · Context: #39}

[wrokin]

🛡️ wrokin security hunter

No vulnerabilities were proven in this pull request — the hunter found nothing it could exploit and prove by execution.

Advisory only — this check never blocks your PR.