feat(stack)!: align wasm-inline to the Result contract, and add bulkEncrypt/bulkDecrypt

[coderdan]

Closes #737. #737.

Two changes to @cipherstash/stack/wasm-inline, the second of which is breaking:

1. bulkEncrypt / bulkDecrypt — a list of encrypted rows now costs one ZeroKMS round trip instead of one per row.
2. Every fallible method returns a Result — { data } | { failure } — instead of throwing.

1. Result alignment (breaking)

encrypt, decrypt, encryptQuery and encryptQueryBulk threw on failure and returned bare values on success. They now return { data } | { failure }, with failure.type from EncryptionErrorTypes (EncryptionError vs DecryptionError) and failure.code carrying the FFI code.

// before
const encrypted = await client.encrypt(plaintext, { table: users, column: users.email })

// after
const result = await client.encrypt(plaintext, { table: users, column: users.email })
if (result.failure) throw new Error(result.failure.message)
const encrypted = result.data

This is what AGENTS.md:169 requires — "Operations return { data } or { failure }. Preserve this shape and error type values in EncryptionErrorTypes" — and what the native entry has always done. The WASM entry never followed it.

It was drift, not a design decision. Nothing about WASM prevents it: @byteslice/result is already bundled into dist/wasm-inline.js (tsup.config.ts noExternal). The cost was that edge code had to be written in a different shape from every other surface, with failures that were easy to miss.

Why now: it's breaking, and 1.0.0 hasn't shipped. @cipherstash/stack@latest is still 0.19.0, so this surface has only ever been published under the rc tag — there are no stable-tag consumers. After GA this would have waited for a major.

isEncrypted is unchanged: a pure predicate with nothing to fail at, as on the native entry.

2. Bulk operations

// Write: several columns across many rows, one round trip
const encrypted = await client.bulkEncrypt([
  { plaintext: "alice@example.com", table: users, column: users.email },
  { plaintext: "hello", table: users, column: users.bio },
])

// Read: a whole page in one call
const emails = await client.bulkDecrypt(rows.map((r) => r.email))

The entry previously had no bulk operations at all, so an N-row list on Deno / Workers / Supabase Edge meant N sequential ZeroKMS calls — impractical alongside the cold start, and no access to the bulk path AGENTS.md names for throughput. The FFI primitives already existed (encryptBulk, decryptBulkFallible, identical in protect-ffi 0.29 and 0.30); this wires them up.

Index-aligned with the input; null / undefined yields null at the same index without reaching ZeroKMS. Per-item table/column routing is what makes the saving real — one call covers several columns across many rows, where a single-column batch (the native shape) would still cost a round trip per column.

Failure modes handled rather than assumed away

• Partial decrypt failure. bulkDecrypt uses the fallible primitive, so failure.message names every failing index and reason, instead of surfacing the first and discarding work already paid for.
• Length mismatch. All three batch methods assert the FFI returned as many results as were sent. Matching is positional (no correlation id), so a short response would silently leave trailing slots null — indistinguishable from "this row had no value". Silent wrong data, so it fails instead.

Scope: model helpers stay Node-only

#737 also mentions the *Models variants. Deliberately not here — the WASM entry has no encryptModel / decryptModel at all, so adding bulkEncryptModels alone would be incoherent. Tracked as #742.

Validation

• 24 new tests. wasm-inline-result-contract.test.ts pins the Result shape on both paths for all six methods and both error types; wasm-inline-bulk.test.ts covers one-FFI-call-per-batch, payload shape, mixed tables/columns, position stability, short-circuits, per-index failure reporting, and the length guards.
• 831 tests pass. Zero new tsc errors — verified by diffing the error set against main, not by counting (the pre-existing 107 are unchanged). 0 biome errors. Build clean.

Found while testing: withResult's ensureError replaces any non-Error throw with new Error('Something went wrong') before the error mapper runs, discarding the original value (@byteslice/result@0.2.0, dist/result.js:27). That's library-wide — the native entry has it too — so it's pinned as a test rather than worked around locally, and will surface if the #532 bump to 0.5.0 changes it.

Consumers updated

The supabase-worker example (now returns a 500 with the failure message rather than throwing past the handler) and the WASM integration adapter (via an unwrap helper that aborts the harness loudly). Skill updated — stash-encryption documented only the native bulk shape, which wouldn't compile on the edge.

Note: if you see SteVec/bloom failures locally, that's a stale install — package.json requires protect-ffi 0.30.0 since #711, and pnpm install --frozen-lockfile fixes it with no lockfile change.

Summary by CodeRabbit

Summary by CodeRabbit

• New Features
  • Added bulk encryption/decryption for the WASM entry, including index-aligned handling of null/undefined inputs.
• Breaking Changes
  • Updated encryption/decryption query operations to return { data } | { failure } results (instead of throwing), with structured failure details.
• Documentation
  • Documented WASM bulk payload format, null-value passthrough, aggregated bulk decrypt failure reporting, and WASM-only helper availability.
• Tests
  • Added/updated WASM bulk, result-contract, and helper tests to validate batching behavior and failure aggregation.

Summary by CodeRabbit

• New Features
  • Added bulk encryption and decryption for WASM clients with index-aligned results.
  • Added support for null and undefined batch entries without unnecessary processing.
  • Bulk decryption now reports all failed item indices and reasons together.
• Improvements
  • Fallible WASM operations now return structured { data } or { failure } results.
  • Enhanced error details include failure types and available error codes.
  • Updated documentation and examples to reflect the new WASM behavior.

[changeset-bot]

🦋 Changeset detected

Latest commit: 1ffdc56

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
@cipherstash/stack	Minor
stash	Minor
@cipherstash/bench	Patch
@cipherstash/prisma-next	Minor
@cipherstash/stack-drizzle	Minor
@cipherstash/stack-supabase	Minor
@cipherstash/test-kit	Patch
@cipherstash/basic-example	Patch
@cipherstash/prisma-next-example	Patch
@cipherstash/e2e	Patch
@cipherstash/wizard	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[coderabbitai]

📝 Walkthrough

Walkthrough

The WASM encryption client now returns typed Result values for fallible operations and supports index-aligned bulkEncrypt and bulkDecrypt through bulk FFI calls, with updated integrations, tests, release notes, and documentation.

Changes

WASM Result and bulk operations

Layer / File(s)	Summary
Result contract and client wiring packages/stack/src/wasm-inline.ts	Fallible encryption, decryption, and query methods now return { data } | { failure }; bulk methods filter nullish inputs, preserve indexes, validate result lengths, and aggregate decryption failures.
Bulk operation validation packages/stack/__tests__/wasm-inline-bulk.test.ts, packages/stack/__tests__/helpers/*	Tests and FFI stubs cover one-call batching, payload shapes, nullish handling, failure aggregation, and mismatched result lengths.
Result consumers and contract tests packages/stack/integration/wasm/adapter.ts, examples/supabase-worker/..., packages/stack/__tests__/wasm-inline-result-contract.test.ts, packages/stack/__tests__/helpers/expect-result.ts	WASM integrations unwrap failures, the edge example handles success and failure results, and contract tests verify typed failures and success envelopes.
Query coverage and published behavior packages/stack/__tests__/wasm-inline-query.test.ts, .changeset/*, skills/stash-encryption/SKILL.md	Query tests, release notes, and documentation describe the Result contract, bulk behavior, and Node-only model helpers.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant WasmEncryptionClient
  participant decryptBulkFallible
  participant ZeroKMS
  Caller->>WasmEncryptionClient: bulkDecrypt(ciphertexts)
  WasmEncryptionClient->>decryptBulkFallible: live ciphertext payloads
  decryptBulkFallible->>ZeroKMS: one bulk decrypt request
  ZeroKMS-->>decryptBulkFallible: success and failure results
  decryptBulkFallible-->>WasmEncryptionClient: fallible result array
  WasmEncryptionClient-->>Caller: aligned plaintexts or aggregated failure

Loading

Possibly related issues

• wasm-inline has no model helpers: encryptModel/decryptModel (and their bulk forms) are Node-only #742: Documents the WASM model-helper limitation that this changeset explicitly retains.

Possibly related PRs

• cipherstash/stack#496: Established the WASM inline client extended by these changes.
• cipherstash/stack#676: Modified the WASM query APIs that now adopt the Result contract.

Suggested reviewers: tobyhede

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR adds bulkEncrypt and bulkDecrypt with the required batch semantics, addressing the linked issue's core need.
Out of Scope Changes check	✅ Passed	The Result-contract, tests, docs, adapter, and example updates support the same WASM API change and are not unrelated scope.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: wasm-inline now uses the Result contract and adds bulkEncrypt/bulkDecrypt.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dan/cip-3584-wasm-inline-bulk-ops

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/stack/src/wasm-inline.ts (1)

612-618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate live-item partitioning logic across bulk methods.

The filter-into-live/pre-fill-out-with-null/short-circuit-on-empty pattern is now written three times in this file: encryptQueryBulk (541-547, pre-existing), bulkEncrypt (612-618), and bulkDecrypt (683-689). Since assertBatchLength was already extracted as a shared helper for the positional-safety contract, extracting this partitioning step too would keep all three bulk methods' index-alignment logic in one place, reducing the risk of the pattern silently diverging if one method's null-handling changes later.

♻️ Example extraction

function partitionLive<T, L>(
  items: readonly T[],
  isLive: (item: T) => boolean,
): { live: Array<{ item: T; at: number }>; outTemplate: Array<L | null> } {
  const live: Array<{ item: T; at: number }> = []
  items.forEach((item, at) => {
    if (isLive(item)) live.push({ item, at })
  })
  return { live, outTemplate: items.map(() => null) }
}

Also applies to: 683-689

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stack/src/wasm-inline.ts` around lines 612 - 618, Extract the
repeated live-item partitioning and null-output initialization into a shared
helper near the bulk methods, such as partitionLive, accepting the items and a
liveness predicate and returning live entries plus an output template. Update
encryptQueryBulk, bulkEncrypt, and bulkDecrypt to use this helper while
preserving their existing empty-live short-circuit and index alignment behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/stack/src/wasm-inline.ts`:
- Around line 612-618: Extract the repeated live-item partitioning and
null-output initialization into a shared helper near the bulk methods, such as
partitionLive, accepting the items and a liveness predicate and returning live
entries plus an output template. Update encryptQueryBulk, bulkEncrypt, and
bulkDecrypt to use this helper while preserving their existing empty-live
short-circuit and index alignment behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8861492a-6efa-4fbd-9e0d-42ff760d924f

📥 Commits

Reviewing files that changed from the base of the PR and between 0e96d36 and ffadd95.

📒 Files selected for processing (5)

• .changeset/wasm-inline-bulk-ops.md
• packages/stack/__tests__/helpers/stub-protect-ffi-wasm-inline.ts
• packages/stack/__tests__/wasm-inline-bulk.test.ts
• packages/stack/src/wasm-inline.ts
• skills/stash-encryption/SKILL.md

[coderabbitai]

🧹 Nitpick comments (1)

packages/stack/src/wasm-inline.ts (1)

598-629: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider extracting the shared bulk scaffolding.

encryptQueryBulk, bulkEncrypt, and bulkDecrypt all repeat the same pattern: filter live inputs into { item, at }, seed an all-null aligned out, short-circuit on empty, assertBatchLength, then write results back positionally by slot.at. A small generic helper (taking the liveness predicate, the FFI call, and a per-slot writer) would remove the triplication while keeping bulkDecrypt's failure aggregation as the writer's responsibility. Optional given the divergence in the decrypt path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stack/src/wasm-inline.ts` around lines 598 - 629, The bulk methods
encryptQueryBulk, bulkEncrypt, and bulkDecrypt duplicate live-item filtering,
null-aligned output initialization, empty-input handling, batch-length
validation, and positional result mapping. Extract this shared scaffolding into
a small generic helper parameterized by the liveness predicate, FFI operation,
and per-slot writer; keep bulkDecrypt’s failure aggregation within its writer,
and update each method to use the helper without changing behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/stack/src/wasm-inline.ts`:
- Around line 598-629: The bulk methods encryptQueryBulk, bulkEncrypt, and
bulkDecrypt duplicate live-item filtering, null-aligned output initialization,
empty-input handling, batch-length validation, and positional result mapping.
Extract this shared scaffolding into a small generic helper parameterized by the
liveness predicate, FFI operation, and per-slot writer; keep bulkDecrypt’s
failure aggregation within its writer, and update each method to use the helper
without changing behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: df90ce52-d222-4c71-88a9-1bbcbe230720

📥 Commits

Reviewing files that changed from the base of the PR and between ffadd95 and bbd5b18.

📒 Files selected for processing (9)

• .changeset/wasm-inline-bulk-ops.md
• examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts
• packages/stack/__tests__/helpers/expect-result.ts
• packages/stack/__tests__/wasm-inline-bulk.test.ts
• packages/stack/__tests__/wasm-inline-query.test.ts
• packages/stack/__tests__/wasm-inline-result-contract.test.ts
• packages/stack/integration/wasm/adapter.ts
• packages/stack/src/wasm-inline.ts
• skills/stash-encryption/SKILL.md

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/stack/tests/wasm-inline-bulk.test.ts

[freshtonic]

Overview

Two changes to @cipherstash/stack/wasm-inline: (1) every fallible method now returns { data } | { failure } instead of throwing — aligning the edge surface with the native entry and the AGENTS.md Result contract (breaking, but the surface has only ever shipped under the rc tag, so no stable consumers); (2) adds bulkEncrypt/bulkDecrypt, collapsing an N-row list from N ZeroKMS round trips to one — the thing that made list endpoints impractical on Deno/Workers/Edge.

Carefully implemented and, importantly, correct at the FFI boundary. LGTM / approvable — details below; no blocking issues.

What I verified

• Result contract is correct and at parity with the native entry. Each method routes to the right error type: encrypt/encryptQuery/encryptQueryBulk/bulkEncrypt → EncryptionErrorTypes.EncryptionError; decrypt/bulkDecrypt → DecryptionError. toFailure produces { type, message, code: getErrorCode(error) } — the same shape the native bulk-decrypt.ts failure mapper emits. isEncrypted correctly stays a plain boolean (pure predicate), matching native.
• The FFI boundary contracts all match the native reference — the one place a stub-based test can pass while disagreeing with the real FFI, so I checked against packages/protect: encryptBulk payload field is plaintexts (matches native model-helpers.ts); decryptBulkFallible payload field is ciphertexts (matches native bulk-decrypt.ts); and the per-item result discriminant ('error' in result) ? result.error : result.data is exactly what native mapDecryptedDataToResult uses. No boundary drift.
• Bulk logic is right. Both methods filter live (non-null) entries carrying their original index, pre-fill an index-aligned null output, short-circuit with no FFI call when the batch is all-null, and scatter results back by position. null/undefined never reaches ZeroKMS. Per-item table/column routing (vs the native single-column batch) is what makes the round-trip saving real, and it's free at the FFI boundary since EncryptPayload is per-item.
• Fail-closed length guard. assertBatchLength on all three batch methods (including the pre-existing encryptQueryBulk, which previously left trailing slots silently null) turns a short FFI response — which positional matching would otherwise render as indistinguishable-from-"no value" nulls — into a { failure }. Correct call: silent wrong data is the worse outcome.
• Partial-decrypt aggregation is thorough. bulkDecrypt uses the fallible primitive and collects every failing index+reason before throwing (caught by withResult → one { failure }), rather than surfacing the first and discarding work already paid for. The [at] indices are correctly into the input array (via slot.at), not the compacted live array.
• Changeset (@cipherstash/stack minor + stash patch for the skill) is appropriate; the breaking change is justified pre-1.0 (rc-tag-only surface). Skill, the supabase-worker example, and the WASM integration adapter are updated for the new shape.

Notes — non-blocking

1. Per-item codes are dropped from the structured output on partial decrypt failure. decryptBulkFallible returns { error, code? } per item, but only result.error (the message) is aggregated; the final failure.code comes from the synthesized aggregate Error, which has none. Reasonable — a batch has no single code and the reasons are in the message — but if callers ever want to branch on per-row codes, they aren't there. The author already flags a per-item Result[] as a possible future shape; that would also address this.
2. ensureError quirk is documented, not worked around — @byteslice/result@0.2.0 replaces a non-Error throw with new Error('Something went wrong') before the mapper runs, so toFailure's error instanceof Error ? … : String(error) narrowing can't actually see a raw non-Error value today. It's library-wide (native has it too) and pinned as a test that will surface if the #532 bump to 0.5.0 changes it. Good handling; noting for awareness.
3. Local node_modules has protect-ffi 0.23/0.26/0.27, not 0.30 in this checkout — the "stale install" the PR calls out. Doesn't affect the diff (the bulk primitives are identical across 0.29/0.30 per the PR), but it means a local pnpm test here wouldn't exercise the real 0.30 FFI without pnpm install --frozen-lockfile first. CI is the real signal.

Verdict

Correct, careful, and verified where it matters most — the FFI field names and per-item result shapes all match the native implementation, the Result routing is right per operation side, and the batch length/partial-failure handling is fail-closed. The notes are non-blocking. I'd approve this — say the word and I'll flip it to a formal approval.

[coderdan]

Follow-up: onException — the Result conversion was losing error detail

Pushed 93a83a3.

Reviewing the earlier ensureError note more carefully, the conclusion was wrong: it isn't a library limitation, it's our code. withResult prefers a hook we never pass:

const error = hooks?.onException?.(ex) ?? ensureError(ex)

ensureError — which replaces a non-Error throw with Error("Something went wrong") — is only the fallback. grep -rn "onException" packages/ returns nothing: no call site in the repo supplies it.

That made this PR's Result conversion a partial regression. The old throwing behaviour propagated the raw rejection value to the caller; withResult was silently swallowing it. And it matters more here than on the native entry, where the FFI throws real ProtectError instances:

• wasm-bindgen rejects with the raw JsValue from Rust — throw takeFromExternrefTable0(ret[0]) in the generated glue.
• the WASM build exports no ProtectError class (dist/wasm/protect_ffi.d.ts), unlike the Node entry.

So a genuine FFI failure on this path can arrive as a bare string or object, and was being replaced with boilerplate.

Fix: all six call sites now go through one wasmResult() seam binding both the failure shape and the onException hook. A method added later cannot omit either — and an omission wouldn't fail a build, it would just quietly degrade messages, which is exactly the bug. toError keeps strings as-is, JSON-serializes objects so {code, detail} survives rather than collapsing to [object Object], and falls back to String() for cycles.

+3 tests (string, object, cyclic). 833 pass, 0 biome errors, zero new tsc errors.

Two pre-existing findings filed separately

• withResult: no call site passes onException, so non-Error throws lose their message repo-wide #743 — no withResult call site anywhere passes onException, so this same detail loss applies repo-wide. The DynamoDB operations already work around it by hand (throwPreservingCode).
• dist/wasm-inline.js imports the NODE @cipherstash/protect-ffi, defeating the entry's purpose #744 — dist/wasm-inline.js imports the Node @cipherstash/protect-ffi alongside the wasm one, defeating the entry's stated purpose. Pre-existing — I verified by building main, it is not from this PR. Nothing guards it: prisma-next has a bundling-isolation test, stack has no equivalent for the WASM entry.

[coderdan]

Deep review — 15 findings

The core implementation checks out: FFI routing, payload shapes, error classification, and positional matching are all correct, and the new unit suites pass 36/36 in isolation. The critical issue is a transitive import that breaks the entry on the very runtimes it targets (inline comment on src/wasm-inline.ts:89).

10 findings are inline on the diff (tagged by rank). The 5 below anchor outside the diff hunks:

2/15 — Deno e2e suites not migrated to the Result contract

e2e/wasm/roundtrip.test.ts and e2e/wasm/query-types.test.ts still consume bare values. With live CS_* credentials: isEncrypted(encrypted) (roundtrip:111) receives { data } and fails; roundtrip:141 passes the Result envelope to decrypt as ciphertext; bulk.length (roundtrip:165, query-types:~228) is undefined on a Result; every assertV3Term(await client.encryptQuery(...)) sees term.v === undefined. The suites are secret-gated so this lands silently — and the unit tests' claim that “live coverage runs in the Deno e2e” has no backing for the new bulk ops either (no bulkEncrypt/bulkDecrypt calls in e2e/wasm/). Separately, the e2e import map (e2e/wasm/deno.json) maps only the three /wasm-inline subpaths, so the new bare @cipherstash/protect-ffi specifier in the built entry (finding 1) is unresolvable there.

6/15 — pending changeset now contradicts this PR

.changeset/wasm-encrypt-query.md:17 still says “Errors throw, consistent with the WASM surface's encrypt/decrypt”. Both changesets roll into the same GA changelog entry when pre mode exits — one tells users encryptQuery throws, the adjacent one says every fallible method returns a Result. Update the stale sentence in this PR.

7/15 — the entry doesn't export its own failure vocabulary

Every method is now typed Result<T, EncryptionError> and the changeset directs users to discriminate failure.type against EncryptionErrorTypes — but the entry (export block at src/wasm-inline.ts:121) exports neither EncryptionError, EncryptionErrorTypes, nor Result. An edge consumer writing switch (result.failure.type) { case EncryptionErrorTypes.DecryptionError: ... } must reach for the Node-oriented @cipherstash/stack/errors subpath (undocumented on edge) or depend on the bundled-internal @byteslice/result — contradicting the entry's single-import design.

11/15 — the same latent flaws stay unfixed on the native entry

The onException fix and assertBatchLength guard exist only here. Every native withResult call (packages/stack/src/encryption/operations/*, packages/protect/src/ffi/operations/*) omits onException, so a non-Error rejection still becomes “Something went wrong” — the exact loss this PR documents and fixes for WASM. And native bulk ops match FFI results positionally unguarded: a short response makes decryptResult undefined and 'error' in decryptResult (packages/stack/src/encryption/operations/bulk-decrypt.ts:49) throws a bewildering TypeError. Native bulkDecrypt also preserves per-item partial success that the WASM one collapses — same-named op, divergent failure contract. Deeper fix: hoist toError/toFailure/assertBatchLength into @/encryption/helpers and use them from both entries.

12/15 — Linear ID in public artifacts

Commit ffadd95b's message and the PR title carry the internal Linear ID; the changeset, skill, and source comments are clean (#737/#741). Reword the commit via rebase and drop the suffix from the PR title before merge — after merge it's permanent.

---

Review method: 11 independent finder angles → dedup → per-candidate verification against the built artifact, FFI typings, and files on disk → gap sweep. Findings 9/15 and 10/15 are PLAUSIBLE (mechanism verified, trigger uncommon); all others CONFIRMED.

[coderdan]

1/15 · critical — this import breaks the entry on every runtime it exists for.

@/encryption/helpers/error-code does a runtime value-import of ProtectError from the native @cipherstash/protect-ffi root entry, and protect-ffi is not in tsup noExternal — so the built dist/wasm-inline.js now contains a bare import { ProtectError as FfiProtectError } from "@cipherstash/protect-ffi" (verified by building).

• On Cloudflare Workers / Supabase Edge the non-node default condition resolves that to dist/wasm/protect_ffi.js, which exports no ProtectError → missing-named-export error at build/link time; it also top-level-imports a raw .wasm asset — the loading mode this entry exists to avoid.
• Under Deno's node condition it resolves to the NAPI loader — the native-module dependency this entry exists to avoid. The e2e import map (e2e/wasm/deno.json) maps only the /wasm-inline subpaths, so the bare specifier is unresolvable there at all.

The unit tests can't see this: they mock only the /wasm-inline subpath and run under Node, where the native root resolves fine.

Fix: don't import the native-entry helper here — duck-type the code lookup (read .code structurally, with a type-only import of ProtectErrorCode). Worth adding an import-graph assertion on dist/wasm-inline.js so this class of regression fails CI.

[coderdan]

3/15 — failure.code can never be populated on this entry, contradicting the changeset's “failure.code carrying the FFI error code where there is one”.

withResult runs onException (toError) first, so by the time toFailure runs, error is always a fresh plain Error — and getErrorCode only matches instanceof the native ProtectError class, which the WASM build doesn't export and wasm-bindgen rejections never are. A rejection of { code: 'EQL_X', detail: '…' } gets its code JSON-flattened into message, never surfaced in failure.code.

Relatedly, bulkDecrypt (line ~829) discards the per-item code the FFI does supply — the real arm is { error: string; code?: ProtectErrorCode } per protect-ffi's lib/index.d.cts — so callers can't branch on error codes (retry vs corrupt payload) anywhere on this entry. No test asserts failure.code, so the dead field ships pinned as passing; the result-contract test even rejects with a code-bearing object but only asserts message.

[coderdan]

4/15 — as committed, this example can't run against any published version.

• It still pins npm:@cipherstash/stack@^0.18.0/wasm-inline (line 21) — a version whose entry returns bare values. Deployed as-is: encrypt returns the payload, encryptResult.failure is undefined (check passes), encryptResult.data is undefined → decrypt(undefined) throws → 500 on every request.
• Bump the pin and it breaks earlier: the example authors an EQL v2 schema (encryptedColumn('email').equality(), line 24), but the current entry exports only the v3 surface (export * from '@/eql/v3' — no encryptedColumn) and its factory rejects non-v3 tables outright.

examples/supabase-worker/README.md:62 still advertises encryptedColumn on this surface too. The example needs the version bump and the v3 schema migration together.

[coderdan]

5/15 — the published TSDoc still teaches the pre-#741 contract.

• This doc block now has two contradictory @returns tags — this stale bare-value line survived above the new Result-shaped one. Generated docs and IDE hovers render both.
• The module-header example (lines 34–49) still does const dec = await client.decrypt(enc) on a bare encrypt result and interpolates ${term} straight into SQL — copying the entry's own headline example passes the { data } envelope into decrypt (guaranteed failure) and binds {"data":{…}} as the jsonb query term.
• encryptQuery's four @example blocks (lines 541–584) interpolate the un-unwrapped return into SQL the same way.

Every method signature was migrated but the examples weren't — and they ship in the published .d.ts.

[coderdan]

8/15 — this section is unusable in the skill's own context, and skills ship into customer repos.

At this point in the document the running client is the native EncryptionV3 client; the skill never shows constructing a WASM client (the Encryption factory from @cipherstash/stack/wasm-inline appears nowhere in the file), and the Subpath Exports table (~line 183) doesn't list @cipherstash/stack/wasm-inline at all.

Concrete failure: a customer-repo agent applies this snippet to the client it has in scope — the native one — calling client.bulkEncrypt([{ plaintext, table, column }]). The native signature is bulkEncrypt(plaintexts, { table, column }), so the items are consumed as { id?, plaintext } envelopes with no column argument and the call fails at runtime in the customer's codebase.

Add the WASM client construction (import + Encryption({ schemas, config })) to this section and the subpath to the exports table.

[coderdan]

9/15 (PLAUSIBLE) — this fallback can itself throw, escaping the Result contract.

String(ex) throws for a null-prototype object (Object.create(null) — no toString), and withResult runs onException bare inside its catch (options?.onException?.(ex) ?? ensureError(ex) — no inner try, verified in the @byteslice/result source). So: FFI rejects with a cyclic null-prototype object → JSON.stringify throws on the cycle → this catch → String(ex) throws TypeError: Cannot convert object to primitive value → propagates out of withResult → client.encrypt(...) rejects, and edge code written as if (r.failure) crashes with an unhandled rejection.

Trigger is contrived, but the whole point of toError is being the last line of defence. Guard it: try { return new Error(String(ex)) } catch { return new Error('unserializable rejection') } (or use Object.prototype.toString.call(ex), which can't throw).

[coderdan]

10/15 (PLAUSIBLE) — sparse-array holes yield undefined, not the documented null.

Array.prototype.map and forEach both skip holes: for bulkDecrypt([ct1, , ct2]) (e.g. a partially-filled new Array(n)), forEach correctly excludes index 1 from the FFI call, but this map also skips it, leaving out[1] a hole — result.data[1] is undefined, violating the “null/undefined entries yield null at the same index” contract while lengths still match. Same pattern in bulkEncrypt (line ~728) and encryptQueryBulk (line ~653).

Fix: Array.from({ length: items.length }, () => null) for the out array (holes in the input are still correctly treated as dead slots by the forEach filter).

[coderdan]

13/15 — the harness still pays fields × rows round trips, and the new bulk path gets zero live coverage anywhere.

encryptRow issues one client.encrypt per field inside Promise.all (concurrency hides latency, not request count — a 100-row × 5-field family is 500 ZeroKMS requests). Meanwhile the unit tests mock the FFI (“live coverage runs in the Deno e2e”) and e2e/wasm/ contains no bulkEncrypt/bulkDecrypt calls — so nothing in the repo exercises the new bulk ops against real ZeroKMS.

Flattening each row's (or the whole batch's) fields into one client.bulkEncrypt call — its per-item { plaintext, table, column } routing exists precisely for this shape — collapses N×F requests into 1 and makes this harness the live coverage the PR currently lacks. The comment above (“rather than paying fields × RTT per row”) is stale either way.

[coderdan]

14/15 — two coverage gaps that would let silent-data-loss regressions through:

1. No falsy-but-live values. Position-stability is only pinned with null/undefined vs truthy strings. If any of the three live-filters (src/wasm-inline.ts:647/725/802) regressed from !== null && !== undefined to a truthiness check, bulkEncrypt([{ plaintext: 0 }, { plaintext: '' }, { plaintext: false }]) would return { data: [null, null, null] } without contacting ZeroKMS — real values persisted as NULL — with every test green. Add a case asserting 0/''/false reach the FFI and return at their indices.
2. encryptQueryBulk's assertBatchLength is untested. The length-mismatch describe below covers bulkEncrypt/bulkDecrypt only; dropping the guard at src/wasm-inline.ts:665 would pass the whole suite.

(Also minor: the two mismatch test titles still say “throws rather than returning…” while the bodies assert .resolves.toMatchObject({ failure }) — leftovers from the pre-Result commit.)

[coderdan]

15/15 (cleanup) — the positional-batch scaffolding is now hand-rolled three times, and the PR adds a fourth Result-unwrap helper variant.

• The live-slot build / null pre-fill / empty short-circuit / assertBatchLength / positional scatter (~13 lines) is triplicated across encryptQueryBulk, bulkEncrypt, and bulkDecrypt — the exact code this PR deems subtle enough to need a guard. A future batch method (e.g. the model-helper port) can omit assertBatchLength with no build failure — the same silent-omission risk the wasmResult docstring says that helper exists to prevent, solved for error wrapping but not batching. One compactBatch(items, isLive, send) helper owning bookkeeping + length assert + scatter fixes it (and the dead if (!slot) guards after assertBatchLength go away with it).
• integration/wasm/adapter.ts's new unwrap() and __tests__/helpers/expect-result.ts's expectData duplicate unwrapResult, which already exists in @cipherstash/test-kit (whose docblock says it exists precisely to stop per-file unwrap() re-declarations) and in this package's own __tests__/fixtures/index.ts:75.

[coderdan]

Thanks — that review found a genuine blocker I'd missed, and one I'd actively mis-diagnosed. All 15 addressed across c70b823d..1ffdc562.

1/15 — critical, and I had this wrong before

You're right, and my earlier dismissal of it was based on an invalid test. I'd run git stash -u to swap in main, but the tree was already clean — so it stashed nothing, and I rebuilt my own branch and compared it against itself. Re-verified properly by swapping in main's wasm-inline.ts and rebuilding in place:

mine:  @cipherstash/protect-ffi   +  @cipherstash/protect-ffi/wasm-inline
main:                                @cipherstash/protect-ffi/wasm-inline

Replaced with a structural readErrorCode — which, as you note, is the only thing that could ever have worked there: the WASM build ships no error class for instanceof to match.

Added wasm-inline-bundle-isolation.test.ts asserting the built bundle's external imports, and verified it catches the regression by reintroducing it (2 of 3 assertions fail) rather than assuming. I'd also filed #744 on the faulty premise — corrected and closed.

The rest

#	Fix
2	Both Deno e2e suites unwrap at every boundary; roundtrip gains the first live bulkEncrypt/bulkDecrypt coverage in the repo, including the null-hole. (deno check's column-brand errors are pre-existing — the task runs --no-check deliberately, per deno.json. Confirmed no new error kinds.)
3	toError now carries a structural code onto the synthesized Error, so failure.code can populate at all; bulkDecrypt puts each item's FFI code in its per-index message.
4	Example pinned ^1.0.0-rc.3 and migrated to types.TextEq — you were right that the bump alone breaks it earlier. README claim updated.
5	Duplicate @returns removed; module-header and all five encryptQuery examples migrated off the pre-Result contract.
6	.changeset/wasm-encrypt-query.md sentence corrected.
7	EncryptionError / EncryptionErrorTypes now exported from the entry.
9	safeString guards the null-prototype case — you were right that it escapes the Result contract entirely, since withResult calls onException bare inside its catch.
10	Array.from, not map.
12	Commit reworded (CIP-… → #737) and dropped from the PR title. Worth noting the repo uses both squash and merge commits, so a merge commit really would have carried it onto main — the reword wasn't optional.
13	Adapter now issues one bulkEncrypt per row instead of one request per field.
14	Falsy-but-live (0 / '' / false) per method, the sparse-hole case, encryptQueryBulk's untested guard, and the stale titles.
15	One runBatch helper owns compaction, short-circuit, length assert and scatter.

One thing your finding 7 surfaced indirectly

Exporting the failure vocabulary initially meant re-exporting Result from @byteslice/result — which put that specifier at the top of the emitted .d.ts. A Deno consumer can't resolve it: it's bundled into wasm-inline.js, so it isn't a package they can import, and the e2e import map maps only the three /wasm-inline subpaths.

So WasmResult<T> is now declared locally, for the same reason WasmPlaintext re-declares protect-ffi's JsPlaintext. The published types are self-contained again.

11/15 — filed rather than fixed here

#746. Hoisting toError / toFailure / assertBatchLength into @/encryption/helpers for both entries is the right shape, but it changes the native failure contract (per-item partial success vs collapsed) and belongs in its own reviewable change. #743 already covers the repo-wide onException half.

Validation

840 tests, zero new tsc errors (diffed against main's error set, not counted), 0 biome errors, build clean, bundle verified free of the native FFI.