feat(workspace): link protect-ffi and build it in the jobs that need it - #863
Merged
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe pull request updates CI workflows to build native or WASM Protect FFI bindings, changes selected dependencies to workspace references, adds artifact-aware Protect FFI tests, and centralizes workflow test helpers and validation. ChangesProtect FFI CI and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant CSSecrets
participant BuildFFIBinding
participant Database
participant TestJob
Workflow->>CSSecrets: validate credentials
CSSecrets->>BuildFFIBinding: permit binding build
BuildFFIBinding->>Database: complete binding setup before startup
Database->>TestJob: provide test environment
TestJob->>TestJob: run integration or E2E tests
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
tobyhede
force-pushed
the
feat/protect-ffi-4-workspace-link
branch
from
August 6, 2026 01:06
e0d242e to
98aee99
Compare
freshtonic
approved these changes
Aug 6, 2026
freshtonic
left a comment
freshtonic
left a comment
Contributor
There was a problem hiding this comment.
Review: APPROVE
Purely CI-wiring + internal dependency linking (stack 4 of 4). Consumers move @cipherstash/protect-ffi from the 0.31.0 pin to workspace:*, and every job that encrypts/decrypts/typechecks against it gains build-ffi-binding. Reviewed in context against the base branch; ran everything runnable locally.
Verification performed
pnpm install --frozen-lockfile— clean, lockfile in sync.pnpm run test:scripts— 295 passing, incl. newffi-binding-step-order.test.mjs(9) and the updatedlint-no-workflow-cachingsuite.- protect-ffi
lintWiring.test.ts— 12 passing;pnpm run lint:ffi-changeset— passes (no protect-ffi changeset in the PR). pnpm run code:check(biome) — clean for the PR's files.EXPECTED_PAIRED_JOBSmatches ground truth exactly (7 files, 8 paired jobs).wasm:flags cross-checked against each job'sCS_IT_SUITE: drizzlewasm:'true'(includesintegration/wasm/**) ✓, supabase native-only ✓, prisma-next native-only ✓.wasm-e2e-testsreorder verified:require-cs-secrets→build-ffi-binding(wasm)→test:typecheck:wasm→Build stack→ Deno. Preflight correctly moves up above the builds.
Blocking
None.
Nits (non-blocking)
prisma-example-readme-e2e.ymlcomment says the binding build "is only ever exercised by the push-to-main run", but the protect-ffi source paths were added to bothpushandpull_requestfilters, so a PR touchingpackages/protect-ffi/**triggers it too. Behaviour is correct/more coverage;prisma-next-e2e.yml's wording ("on push to main and on any PR touching its paths") is the accurate version — consider aligning.EXPECTED_PAIRED_JOBSis a hand-maintained list. Deliberate and well-documented ("guard on the scan"), just flagging that future job renames must touch it.
Highlights
ffi-binding-step-order.test.mjsis exemplary — enforces an invisible ordering invariant, discovers paired jobs rather than listing them, guards the scan against vacuous-pass with a mutation-tested floor, and its failure messages teach the correct direction of the fix.- The caching-gate test uses a live citation (
build-ffi-binding/action.yml) and asserts the caching exists before asserting the gate flags it, so it can't rot into proving nothing. build-ffi-bindingverifies the binding actually loads (assertNativeBindingAvailable()+ wasm-inline probe), failing at the named artifact instead of as scattered downstream encryption errors.workspace:*resolves to the same0.31.0the consumers were pinned to, so the "no published surface / no changeset" reasoning holds. No skills impact — no public API/CLI/workflow surface changed.
Solid, exceptionally well-documented work. Approving.
`@cipherstash/stack` and the two adapters move from the exact pin `0.31.0` to `workspace:*`, so they resolve the in-tree copy rather than the npm tarball. This is the step the previous three set up, and it is deliberately atomic. **Why it cannot be split further.** The tarball ships `lib/`, `index.node` and `dist/wasm/**` as CONTENTS. The workspace copy ships them as BUILD OUTPUTS — `lib/` is the package `main` and is generated, and nothing produces `index.node` without cargo. So the moment this line changes, every CI job that encrypts, decrypts, or typechecks against the package needs the binding built first, and the workflow steps below have to land in the same commit. Split them and the failure is `Cannot find module '.../index.node'`, reported once per test rather than once per job. `minimumReleaseAgeExclude` drops its `@cipherstash/protect-ffi*` entry in the same commit and for the same reason: the cooldown applies to registry dependencies, and as of this line these are not registry dependencies. Leaving it would be a stale exemption for packages nothing fetches. The published surface does not change — the workspace copy is the 0.31.0 release, tree-identical to the tag. The `major` changeset for adopting 0.31.0 is in the first PR of this stack, where the breaking change actually is.
The other half of the workspace link. `.github/actions/build-ffi-binding` compiles the binding and proves it loads, and every job that reaches encryption now runs it before its tests: `tests.yml`'s unit, e2e and bun jobs, the three integration suites, and both Prisma e2e workflows. The action caches `index.node` on a content hash of the Rust inputs, so a PR touching no Rust pays a restore rather than a compile. `wasm: 'true'` is passed by exactly two jobs — the ones that load the real WASM. stack's `dist/wasm-inline.js` imports protect-ffi's `dist/wasm/protect_ffi_inline.js` by relative path, and only the three `.d.ts` under `dist/wasm` are tracked; the `.js` and `.wasm` come from wasm-pack and nothing else in CI runs it. **Ordering is load-bearing in two places, and neither looks it.** `require-cs-secrets` must precede `build-ffi-binding`. The secrets check reads four inputs and fails in seconds; a cold binding build costs minutes, and more with `wasm: 'true'`. Put the build first and a job with a rotated credential pays the whole compile before learning it could never have encrypted anything — which is the same as having no pre-flight. Both steps are self-contained `uses:` blocks and swapping them changes nothing about whether the job passes, so `ffi-binding-step-order.test.mjs` checks it by position across every workflow, discovered rather than listed. Note the direction of the fix when it fails: in `wasm-e2e-tests` a `Build stack` step sits between them and consumes `dist/wasm`, so there the pre-flight moves UP, not the build DOWN. In `wasm-e2e-tests`, `Build stack` must follow the binding build, because it now consumes protect-ffi's `dist/wasm` output from the workspace copy rather than from a tarball. `test:typecheck:wasm` returns here, with the job that runs it. It needs `dist/wasm`, so it cannot hang off the default `test`, and `lintWiring`'s exemption list only accepts a carve-out whose name appears in a root workflow — which is why the script was held back until this commit rather than shipped with the package and a promise. The no-caching gate gains a live citation rather than a fixture: `build-ffi-binding` restores two GitHub Actions caches, and its header says publishing workflows must not use it. The test asserts the caching is really there before asserting the gate flags it, so removing the caching fails the test — which is the prompt to fix the header too, rather than leaving a check that passes because it now proves nothing. No changeset: CI wiring, no published surface. A changeset naming any protect-ffi package would fail `scripts/lint-no-ffi-changeset.mjs` regardless — publishing does not move until the trusted-publishing cutover.
Three strands, all in `scripts/__tests__/`, all of the same kind: a CI invariant that nothing enforced, or enforced somewhere too narrow to see the next instance. Shared helpers (`lib/repo-root.mjs`, `lib/workflows.mjs`). Thirteen guards declared `REPO_ROOT` byte-for-byte identically, and five rolled their own workflow discovery under three different names. Copies of a discovery helper are the worst kind to let drift, because a copy that finds nothing does not fail — it passes, having checked nothing. `shared-test-helpers.test.mjs` keeps them the only copies. Credentialed-job coverage (`ffi-binding-step-order.test.mjs`). The ordering checks only ever saw jobs that already paired both actions, so a job that never built the binding was invisible to them — not failing, absent. `tests.yml`'s `e2e-tests` and `run-tests-bun` were exactly that, and `run-tests-bun` carries `continue-on-error` plus `|| true`, so its omission was silent in the job least likely to be looked at. Both now build the binding; the guard is a scan, so the next one is covered the day it lands. Paths-filter parity (`workflow-paths-filter-parity.test.mjs`). GitHub Actions has no YAML anchors, so every filter is written twice, and a one-sided edit is silent in the worst direction: the workflow keeps running on main and stops running on the PR that introduced the change. The check existed but lived inside the integration-workflow suite, whose discovery selects on `CS_IT_SUITE` — two workflows, out of the nine that write the list twice. Generalised and moved; all nine were already identical, so it went green on the first run. `tests-rust.yml` is the one legitimate asymmetry and is named with its reason. Review feedback, on comments rather than code: - Both prisma e2e workflows claimed the binding build was "only ever exercised by the push-to-main run". The protect-ffi paths were added to the `pull_request` filter too, so a PR touching them runs it — and the absorption PR itself touches three entries in that filter, which the "path-filtered away from the absorption PR" half also got wrong. Five comment blocks corrected. `workflow-trigger-comments.test.mjs` derives the claim from the parsed `on:` filters, so it is the YAML that decides, not a spell-check against two known sentences. - The hand-maintained scan guards now print the ids the scan DID find. On the failure they will actually see — a job rename — the developer had the old id and no way to get the new one but to re-derive it from the workflows. The message is returned as a value so a test can assert on it; vitest only evaluates an assertion message on failure, which is how a diagnostic nothing tests gets stripped back out. No changeset: CI configuration and repo tooling, no published surface.
`src/nativeLoading.test.ts` asserted `assertNativeBindingAvailable()` does not throw, justified by "this suite runs where a binary is installed". That premise died with the absorption. `index.node` used to arrive prebuilt inside the npm tarball; as a workspace package it is a cargo output, the six `platforms/*` links are empty until someone compiles one, and this package's `build` is `tsc`. So on a fresh checkout the assertion failed with MODULE_NOT_FOUND — and it failed under root `pnpm test`, which reaches here through `turbo test --filter './packages/*'`, making a Rust toolchain a prerequisite of the whole repo's default test. That is the exact thing the entry-point split exists to prevent, arriving through the tests instead of through the scripts. No skip. The assertion splits on a filesystem fact — every path a build leaves an `index.node` at — and BOTH branches assert. With an artifact present the load must succeed. With none, it must fail as `MODULE_NOT_FOUND` naming the platform package, which is the contract `index.cts` documents and `packages/cli`'s `isNativeBinaryMissing` consumes to turn `stash doctor`'s output into an actionable note rather than a raw stack trace. `process.env.CI` was rejected as the split: it is a claim about which machine this is where the question is whether a build has happened, and it lies both ways — `act` and pre-commit wrappers set it without building, and it says nothing about a developer who ran `build:native` and whose positive case would then never run. That negative case turns out to have been covered by nothing. The CLI's own suite hand-builds its inputs, so it proved the matcher matches a string, never that the string is what the loader raises. It now checks against the real error on every artifact-free run. The regression guard is in `lintWiring.test.ts`, beside the existing "keeps cargo off the default test path", because it is the other half of that rule and the half static analysis cannot see: a test does not name `index.node`, it calls an export that reaches the addon four hops down. It re-runs the suite in a child `vitest run` with every binding artifact made unresolvable through `Module._load` and absent from `fs`, and asserts exit 0. The fs half is not redundant with the loader half — the test above decides which branch it is in by looking at the disk, so the two have to agree or the child fails for the wrong reason. Cost is ~1.1s. It earns it: the failure is invisible where it is introduced. An author who ran `build:native` sees green, and so does CI, which builds the binding before running this suite — so in CI this nested run is the only execution of the artifact-free path. Guarded against passing vacuously by asserting the child ran every test file and that the preload logged more than one pid, which proves it reached the fork workers rather than only the vitest process. `tests.yml`'s `run-tests` justified its binding build by citing this test. It no longer can; the step is still required because only 8 of `packages/stack`'s 120 suites mock protect-ffi, so the comment now says that instead.
tobyhede
force-pushed
the
feat/protect-ffi-4-workspace-link
branch
from
August 6, 2026 04:30
47b455a to
a8e1948
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/protect-ffi/src/lintWiring.test.ts (1)
485-516: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRename
repoRoottopackageRoot.
repoRootis bound toprocess.cwd()and is used ascwdplus forpackages/protect-ffifiles. Since the binding does not validate the workspace root, usepackageRootif the process remains package-local, or add a check and rename it otherwise.🤖 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/protect-ffi/src/lintWiring.test.ts` around lines 485 - 516, Rename the process.cwd() binding from repoRoot to packageRoot in the nested Vitest test and update its uses for cwd and packages/protect-ffi paths, preserving the existing package-local behavior.scripts/__tests__/workflow-trigger-comments.test.mjs (1)
119-121: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard
wfagainst a null parse result.
readWorkflowreturns the result ofyaml.load, which isundefinedfor an empty or comment-only YAML file. Line 120 readswf.ondirectly. Line 176 callspullRequestReach(readWorkflow(relPath))for every discovered workflow at module load, so one such file throws aTypeErrorat import and the whole suite fails to collect instead of reporting a workflow problem.The sibling consumer uses
wf?.jobsinscripts/__tests__/ffi-binding-step-order.test.mjs. Match that style here.🛡️ Proposed change
function pullRequestReach(wf) { - const on = wf.on ?? wf[true] + const on = wf?.on ?? wf?.[true] if (!on || !Object.hasOwn(on, 'pull_request')) return false🤖 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 `@scripts/__tests__/workflow-trigger-comments.test.mjs` around lines 119 - 121, Update pullRequestReach to safely handle null or undefined workflow parse results before accessing workflow properties, matching the optional-chaining style used by the sibling consumer. Preserve the existing false return for workflows without a pull_request trigger and ensure readWorkflow results from empty or comment-only files do not throw during module initialization.
🤖 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.
Inline comments:
In @.github/workflows/tests.yml:
- Around line 499-511: The Bun job must enforce credential validation, binding
compilation, and test success. Add the existing require-cs-secrets pre-flight
before the build-ffi-binding step, remove continue-on-error: true from the job
or relevant step around line 459, and remove the || true masking the Bun test
command near line 530; leave failures blocking this job.
In `@packages/protect-ffi/src/lintWiring.test.ts`:
- Around line 475-477: Update the NODE_OPTIONS construction in the lint wiring
test to wrap the preload path from mkdtempSync in double quotes before passing
it to --require, while preserving the existing optional NODE_OPTIONS filtering
and joining behavior.
In `@scripts/__tests__/ffi-binding-step-order.test.mjs`:
- Around line 135-139: Update credentialedJobs() to record whether
require-cs-secrets exists and its step index, then assert every
EXPECTED_CREDENTIALED_JOBS entry has the preflight before the BUILD_FFI step.
Preserve the existing paired-job test solely as the ordering diagnostic for
paired jobs.
---
Nitpick comments:
In `@packages/protect-ffi/src/lintWiring.test.ts`:
- Around line 485-516: Rename the process.cwd() binding from repoRoot to
packageRoot in the nested Vitest test and update its uses for cwd and
packages/protect-ffi paths, preserving the existing package-local behavior.
In `@scripts/__tests__/workflow-trigger-comments.test.mjs`:
- Around line 119-121: Update pullRequestReach to safely handle null or
undefined workflow parse results before accessing workflow properties, matching
the optional-chaining style used by the sibling consumer. Preserve the existing
false return for workflows without a pull_request trigger and ensure
readWorkflow results from empty or comment-only files do not throw during module
initialization.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e349392b-65a3-46f6-9c17-d4b6f0ea7d2d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.github/workflows/integration-drizzle.yml.github/workflows/integration-prisma-next.yml.github/workflows/integration-supabase.yml.github/workflows/prisma-example-readme-e2e.yml.github/workflows/prisma-next-e2e.yml.github/workflows/tests.ymlAGENTS.mdpackages/protect-ffi/package.jsonpackages/protect-ffi/src/lintWiring.test.tspackages/protect-ffi/src/nativeLoading.test.tspackages/stack-drizzle/package.jsonpackages/stack-supabase/package.jsonpackages/stack/package.jsonpnpm-workspace.yamlscripts/__tests__/bench-index-expressions.test.mjsscripts/__tests__/ffi-binding-action.test.mjsscripts/__tests__/ffi-binding-step-order.test.mjsscripts/__tests__/fixtures/lint-no-workflow-caching/uses-build-ffi-binding.ymlscripts/__tests__/integration-workflow-paths.test.mjsscripts/__tests__/lib/repo-root.mjsscripts/__tests__/lib/workflows.mjsscripts/__tests__/lint-no-dead-package-paths.test.mjsscripts/__tests__/lint-no-ffi-changeset.test.mjsscripts/__tests__/lint-no-workflow-caching.test.mjsscripts/__tests__/no-removed-drizzle-surface.test.mjsscripts/__tests__/no-removed-eql-version-flag.test.mjsscripts/__tests__/rewriter-copies-in-sync.test.mjsscripts/__tests__/shared-test-helpers.test.mjsscripts/__tests__/turbo-skills-inputs.test.mjsscripts/__tests__/workflow-dispatch-job-conditions.test.mjsscripts/__tests__/workflow-node-gyp.test.mjsscripts/__tests__/workflow-paths-filter-parity.test.mjsscripts/__tests__/workflow-trigger-comments.test.mjsscripts/__tests__/workflow-turbo-build-deps.test.mjsscripts/lint-no-workflow-caching.mjs
💤 Files with no reviewable changes (1)
- pnpm-workspace.yaml
auxesis
approved these changes
Aug 6, 2026
Five review threads, each verified against the tree before being acted on. Two were real gaps in what CI actually runs; the rest were hardening. **The WASM bundle could be regenerated with nothing to run it.** `build-ffi-binding` hashes `tsconfig.wasm-errors.json` and `scripts/inline-wasm.mjs` into its WASM cache key, because `build:wasm` ends in `tsc -p` the former and `postbuild:wasm` runs the latter — between them they emit `dist/wasm/protect_ffi_inline.js`. Neither appeared in the `paths:` filter of `integration-drizzle.yml`, the only integration workflow passing `wasm: 'true'` whose suite selection covers `integration/wasm/**`. So a PR editing the inliner rebuilt the bundle and started no job that loads it. Nothing went red; nothing ran. Fixed by listing both in that filter, and pinned by `wasm-build-inputs-paths-filter.test.mjs`, which DERIVES the input list by parsing the cache key rather than copying it — a hand-copied list stops matching the day someone adds an input, and stops matching silently. It reads the `actions/cache` step by its `inputs.wasm` gate rather than its name, and discounts globs under that step's own `path:` (the tracked `dist/wasm/*.d.ts` are hashed output, not input). Writing that guard surfaced a second instance of the same gap in `integration-protect-ffi.yml`, whose `wasm-round-trip` suite loads the bundle while its filter reached `scripts/**` but no root tsconfig. Closed here too, rather than recorded as a known gap: the `KNOWN_UNCOVERED` map ships empty. Its stale check was verified against the real fix, not assumed — with the entry present and the filter corrected it went red naming the map. **The secrets pre-flight was enforced only where it already was.** `ffi-binding-step-order.test.mjs` generated its ordering assertions from the jobs using BOTH actions, so a credentialed job that built the binding with no pre-flight was not failing the check — it was absent from it. `run-tests-bun` was the live instance: it writes all four `CS_*` into `packages/stack/.env`, gained a binding build in this stack, and had no pre-flight. `AGENTS.md` meanwhile claimed both halves scan the workflow directory, which was true of the coverage half only. The scan now runs over credentialed jobs, so a missing pre-flight fails instead of vanishing, and `run-tests-bun` gets the step. `PREFLIGHT_EXEMPT_JOBS` follows the `BINDING_EXEMPT_JOBS` precedent — empty, guarded against going stale, and deliberately separate, because "never loads the binding" and "need not fail fast on a rotated secret" are different claims. Two mutation records in that file's comments were re-run rather than carried forward, and both had drifted. `continue-on-error: true` and the `|| true` around the Bun vitest call were also flagged. Both are left alone: they predate this stack (a200e2b, "ci: add experimental Bun test run") and are what makes that job experimental. Making it blocking is a CI-policy decision, not a rider on binding wiring. The pre-flight comment there says plainly that the job hides its own failures twice over, so the next reader does not have to rediscover it. **Hardening, no behaviour change on any supported platform.** `pullRequestReach` read `wf.on` unguarded while `IN_SCOPE` calls it at module load for every workflow, so one comment-only file would have thrown during collection and taken the suite down instead of reporting a workflow problem. Guarded, with fixtures for both spellings of nothing — js-yaml returns `null` for a comment-only file and `undefined` for an empty one. `NODE_OPTIONS` now quotes the `--require` path: it splits on whitespace, and that path comes from `tmpdir()`, which this file does not choose. Not reachable on Linux or macOS; one character against a failure that would read as "vitest could not start". `repoRoot` in `lintWiring.test.ts` was `process.cwd()`, which vitest sets to the package directory — as its own `ROOT_WORKFLOW_DIR = '../../.github/workflows'` demonstrates by climbing two levels out of it. Renamed `packageRoot`, matching `nativeLoading.test.ts` in the same package. No changeset: CI wiring and tests, no published surface. No skills affected — no public API, CLI command or user-facing workflow changed.
Contributor
Author
Review findings closed in c5fb871Every thread was verified against the tree before being acted on. Two were real gaps in what CI actually runs; the rest were hardening. Replies are on the individual threads — summarising here, including the two nitpicks from the review body that had no thread of their own. Acted onWASM build inputs missing from the trigger filters (@auxesis). The secrets pre-flight was enforced only where it already was (CodeRabbit). Hardening (CodeRabbit): Deliberately not doneRemoving One correction to @freshtonic's reviewThe counts cited ( Verification: |
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
3 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.
Stack 4 of 4 — splitting #858. Base: #862.
packages/protect-ffiand wire its own CIWhat
@cipherstash/stackand the two adapters move from the exact pin0.31.0toworkspace:*, and the CI jobs that reach encryption gain.github/actions/build-ffi-binding. Review found two that did not — see Known gap below.The same commit also wires
lint:ffi-changesetintotests.yml. That guard has existed since #862 and nothing ran it, which is the failure mode the absorption keeps tripping over: a check nothing invokes reads exactly like a check that passes.Why the two halves cannot be split
The npm tarball ships
lib/,index.nodeanddist/wasm/**as contents. The workspace copy ships them as build outputs —lib/is the packagemainand is generated, and nothing producesindex.nodewithout cargo. The moment the dependency line changes, every job that encrypts, decrypts or typechecks against the package needs the binding built first. Split them and the failure isCannot find module '.../index.node', reported once per test rather than once per job.minimumReleaseAgeExcludedrops its@cipherstash/protect-ffi*entry in the same commit and for the same reason: the cooldown applies to registry dependencies, and as of this line these are not registry dependencies.Ordering is load-bearing in two places, and neither looks it
require-cs-secretsmust precedebuild-ffi-binding. The secrets check costs seconds; a cold binding build costs minutes, more withwasm: 'true'. Build first and a job with a rotated credential pays the whole compile before learning it could never have encrypted anything. Both are self-containeduses:blocks and swapping them changes nothing about whether the job passes — henceffi-binding-step-order.test.mjs, checking by position across every workflow, discovered rather than listed. Note the direction of the fix if it fails: inwasm-e2e-testsaBuild stackstep sits between them and consumesdist/wasm, so there the pre-flight moves up, not the build down.Build stackmust follow the binding build inwasm-e2e-tests, because it now consumes protect-ffi'sdist/wasmfrom the workspace copy rather than from a tarball.test:typecheck:wasmreturns here with the job that runs it. It needsdist/wasmso it cannot hang off the defaulttest, andlintWiring's exemption list only accepts a carve-out whose name appears in a root workflow — so the script was held back rather than shipped with a promise.The no-caching gate gains a live citation rather than a fixture:
build-ffi-bindingrestores two Actions caches and its header says publishing workflows must not use it. The test asserts the caching is really there before asserting the gate flags it, so removing the caching fails the test — the prompt to fix the header too, rather than leaving a check that proves nothing.wasm: 'true'is the minority caseAcross the repo three jobs pass it —
integration-drizzle,integration-protect-ffi(from #862), andtests.yml'swasm-e2e-tests— because those are the ones that load the real WASM build; it costs a second cargo build against wasm32.98aee99a's commit message says "exactly two", counting only the jobs that commit touched.AGENTS.mdnow states the rule rather than a number, since the number drifts and the rule does not.Known gap
Review found two credentialed jobs in
tests.ymlthat reach live encryption without building the binding:e2e-tests— runsturbo run test:e2ewith no file filter, soe2e/vitest.config.ts'stests/**/*.e2e.test.tspicks upprisma-example-readme.e2e.test.ts. That suite isdescribe.skipIf(!authConfigured)and un-skips wheneverCS_CLIENT_IDandCS_CLIENT_KEYare set, which this job sets at job level. It drives the README'spnpm startthroughnewClient, and turbo's^buildreaches protect-ffi'stscbut neverindex.node.run-tests-bun— runspackages/stackvitest against live suites;continue-on-error: trueplus|| truemeans it stays green, so it cannot report the breakage it hits.Fix in progress, not in this push: a coverage assertion beside the ordering one in
ffi-binding-step-order.test.mjs, discovering credentialed jobs by scanning the workflow directory rather than by list —run-tests-bunis only visible that way, since its credentials appear solely inside arun:heredoc.Verification
The head of this stack is byte-identical to #858 across all 311 files the absorption touched (verified per-file by blob hash). Main has moved 8 commits ahead since #858 branched; those 29 files are disjoint from the absorption's 311, so nothing of main's newer work is reverted.
Scripts suite 295 passing; protect-ffi's JS chain passing; supply-chain e2e 21 passing; biome 0 errors; lockfile in sync.
No changeset: CI wiring, no published surface — and a changeset naming any protect-ffi package would fail
lint-no-ffi-changeset.mjsregardless.Summary by CodeRabbit
CI Improvements
Bug Fixes
Tests
Documentation